Repository navigation
Conversation
rdc_policy_register() in the client library started a thread to read rdcd's RegisterPolicy stream and never joined it. The thread looked up its group in policy_threads_ on every pass without a lock, while the calling thread inserted the entry only after starting it. Once the stream ended, Read() failed at once on every pass and the loop went on, so a registered "rdci policy --reg" used a whole CPU core after rdcd went away. When rdci exited after unregistering, the handler and its map were freed while the thread could still read them. With the pending policy fix's notification test, the ASan build reported a heap-use-after-free READ in a thread of rdci as it exited in 4 of 10 runs; with this change it reports nothing in 20. Each registration now owns its stream: a stop flag, the gRPC ClientContext and the reader thread, held by shared_ptr in a map under a mutex. The reader uses only its own stream, touches the handler only to start the call, and leaves the loop when the flag is set or Read() fails. rdc_policy_register() inserts the stream before it starts the thread and returns RDC_ST_INSUFF_RESOURCES if the thread cannot start. rdc_policy_unregister() takes the stream out of the map, asks rdcd to end it, then sets the flag, cancels the context and joins the reader, also when rdcd cannot be reached. It also leaves the group free to register again; before, the entry stayed and rdc_policy_register() returned RDC_ST_CONFLICT for the group until the connection closed. Called from a callback, of any group or handle, it must not wait for a reader, so under the same lock the stream moves to a list that the handler's new destructor joins. The destructor joins every reader the handler owns before stub_ goes away; only a reader that runs the destructor itself, which rdc.h does not allow, is detached. rdc.h now says that unregistering and disconnecting wait for a running callback, that a callback may unregister but not disconnect, and lists RDC_ST_CONFLICT and RDC_ST_INSUFF_RESOURCES for rdc_policy_register() on a handle from rdc_connect(). A statement that did nothing, reply.status(), goes. tests/ci/test_fake_gpu_policy_register.py runs against the fake amd-smi. A registered rdci must use less than 0.5 s of CPU in the 2 s after rdcd is killed (it used 2.00 s before this change) and must exit 0 after SIGINT, three times in a row. Through the C API, a callback unregisters its own group, two callbacks unregister each other's group while both run, and rdc_disconnect() returns with a callback registered; a callback that waited for a reader would hang there. Only a sanitizer build sees the use-after-free, so these tests guard it in the ASan job of the sanitizer CI change. The fake-GPU suite passes (109 passed, 24 skipped) in 5 of 6 runs; in the other, rdcd, which does not use the client library, crashed as it shut down in one fault-injection test. tests/ci passes (75). Under ASan, the new tests pass in 5 runs with no report. Signed-off-by: Adam360x <Adam.pryor@amd.com>
This was referenced Oct 22, 2026
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
rdc_policy_register()in the client library started a thread to readrdcd's policy stream and never joined it. Once the stream ended, the loop spun, so a registeredrdci policy --regused a whole CPU core afterrdcdwent away. Whenrdciexited after unregistering, the thread could still read freed memory (ASan: heap-use-after-free READ in 4 of 10 runs). A group also stayed registered afterrdc_policy_unregister(), so registering it again returnedRDC_ST_CONFLICTuntil the connection closed.Technical Details
ClientContextand the reader thread, held byshared_ptrin a map under a mutex. The reader uses only its own stream and leaves when the flag is set orRead()fails.rdc_policy_register()inserts the stream before it starts the thread and returnsRDC_ST_INSUFF_RESOURCESif the thread cannot start.rdc_policy_unregister()takes the stream out of the map, asksrdcdto end it, cancels the context and joins the reader, also whenrdcdcannot be reached, and leaves the group free to register again. Called from a callback, it hands the stream to the handler's destructor to join.rdc.hsays that unregistering and disconnecting wait for a running callback, that a callback may unregister but not disconnect, and lists the new statuses.Issue Tracking
ISSUE ID : #14107
Part of a series of RDC fixes, which #14107 lists. No JIRA ticket.
Test Plan
test_fake_gpu_policy_register.py, against the fake amd-smi: a registeredrdcimust use under 0.5 s of CPU in the 2 s afterrdcdis killed (it used 2.00 s before) and exit 0 after SIGINT, three times in a row. Through the C API, a callback unregisters its own group, two callbacks unregister each other's group while both run, andrdc_disconnect()returns with a callback registered. The use-after-free shows only under ASan; the sanitizer CI job in this series (#14049) runs these tests.Test Result
Fake-GPU suite: 109 passed, 24 skipped.
tests/ci: 92 passed, 6 skipped. Under ASan, the new tests pass with no report.Submission Checklist