[Core] Fix deadlock in garbage collection when holding lock #60014
Conversation
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
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a deferred release mechanism using a background thread to fix a critical deadlock during garbage collection. The approach is sound, and the new test case effectively reproduces the issue and validates the fix. I have identified a potential resource leak where object IDs in the release batch may not be flushed upon worker shutdown. Additionally, I've suggested an improvement to the close method to log a warning if the release thread doesn't terminate gracefully, which will help in debugging potential future issues.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Signed-off-by: redgrey1993 <[email protected]>
RedGrey1993
force-pushed
the
fix/dataclient_deadlock
branch
from
f66a1cb to
cb6e929
Compare
edoakes
reviewed
Jan 9, 2026
Collaborator
edoakes
left a comment
edoakes
left a comment
There was a problem hiding this comment.
Thanks for working on this @RedGrey1993!
ray-gardener
bot
added
core
RedGrey1993
force-pushed
the
fix/dataclient_deadlock
branch
from
755b4b0 to
5f4a5b5
Compare
Signed-off-by: redgrey1993 <[email protected]>
RedGrey1993
force-pushed
the
fix/dataclient_deadlock
branch
from
5f4a5b5 to
5f0e7bb
Compare
Contributor
Author
|
@edoakes Thanks for the review. I've updated the code according to your suggestions. Please review again at your convenience. |
edoakes
approved these changes
Jan 13, 2026
AYou0207
pushed a commit
to AYou0207/ray
that referenced
this pull request
Jan 13, 2026
…ect#60014) ## Description This PR fixes a critical deadlock issue in Ray Client that occurs when garbage collection triggers `ClientObjectRef.__del__()` while the DataClient lock is held. When using Ray Client, a deadlock can occur in the following scenario: 1. Main thread acquires DataClient.lock (e.g., in _async_send()) 2. Garbage collection is triggered while holding the lock 3. GC calls `ClientObjectRef.__del__()` 4. `__del__()` attempts to call call_release() → _release_server() → DataClient.ReleaseObject() 5. ReleaseObject() tries to acquire the same DataClient.lock 6. Deadlock: The same thread tries to acquire a non-reentrant lock it already holds ## Related issues > Fixes ray-project#59643 ## Additional information This PR implements a deferred release pattern that completely avoids the deadlock: 1. Deferred Release Queue: Introduces _release_queue (a thread-safe queue.SimpleQueue) to collect object IDs that need to be released 2. Background Release Thread: Adds _release_thread that processes the release queue asynchronously 3. Non-blocking `__del__`: `ClientObjectRef.__del__()` now only puts IDs into the queue (no lock acquisition) --------- Signed-off-by: redgrey1993 <[email protected]> Co-authored-by: redgrey1993 <[email protected]> Signed-off-by: jasonwrwang <[email protected]>
rushikeshadhav
pushed a commit
to rushikeshadhav/ray
that referenced
this pull request
Jan 14, 2026
…ect#60014) ## Description This PR fixes a critical deadlock issue in Ray Client that occurs when garbage collection triggers `ClientObjectRef.__del__()` while the DataClient lock is held. When using Ray Client, a deadlock can occur in the following scenario: 1. Main thread acquires DataClient.lock (e.g., in _async_send()) 2. Garbage collection is triggered while holding the lock 3. GC calls `ClientObjectRef.__del__()` 4. `__del__()` attempts to call call_release() → _release_server() → DataClient.ReleaseObject() 5. ReleaseObject() tries to acquire the same DataClient.lock 6. Deadlock: The same thread tries to acquire a non-reentrant lock it already holds ## Related issues > Fixes ray-project#59643 ## Additional information This PR implements a deferred release pattern that completely avoids the deadlock: 1. Deferred Release Queue: Introduces _release_queue (a thread-safe queue.SimpleQueue) to collect object IDs that need to be released 2. Background Release Thread: Adds _release_thread that processes the release queue asynchronously 3. Non-blocking `__del__`: `ClientObjectRef.__del__()` now only puts IDs into the queue (no lock acquisition) --------- Signed-off-by: redgrey1993 <[email protected]> Co-authored-by: redgrey1993 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
This PR fixes a critical deadlock issue in Ray Client that occurs when garbage collection triggers
ClientObjectRef.__del__()while the DataClient lock is held.When using Ray Client, a deadlock can occur in the following scenario:
ClientObjectRef.__del__()__del__()attempts to call call_release() → _release_server() → DataClient.ReleaseObject()Related issues
Additional information
This PR implements a deferred release pattern that completely avoids the deadlock:
__del__:ClientObjectRef.__del__()now only puts IDs into the queue (no lock acquisition)