-
Notifications
You must be signed in to change notification settings - Fork 163
Catch IOException with message "Resource deadlock avoided" which can happen on Unix level when multiple process try to lock same file #1799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -161,9 +161,19 @@ private FileLock fileLock(FileChannel channel, boolean shared) throws IOExceptio | |
| try { | ||
| lock = channel.lock(0, Long.MAX_VALUE, shared); | ||
| break; | ||
| } catch (OverlappingFileLockException e) { | ||
| } catch (OverlappingFileLockException | IOException e) { | ||
| // For Unix process sun.nio.ch.UnixFileDispatcherImpl.lock0() is a native method that can throw | ||
| // IOException | ||
| // with message "Resource deadlock avoided" | ||
| // the system call level is involving fcntl() or flock() | ||
| // If the kernel detects that granting the lock would result in a deadlock | ||
| // (where two processes are waiting for each other to release locks which can happen when two processes | ||
| // are trying to lock the same file), | ||
| // it returns an EDEADLK error, which Java throws as an IOException. | ||
| // Read another comment from | ||
| // https://github.com/bdeployteam/bdeploy/blob/7c04e7228d6d48b8990e6703a8d476e21024c639/bhive/src/main/java/io/bdeploy/bhive/objects/LockableDatabase.java#L57 | ||
| if (attempts <= 0) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we add some "filtering" for IOException such? |
||
| throw new IOException(e); | ||
| throw (e instanceof IOException) ? (IOException) e : new IOException(e); | ||
| } | ||
| try { | ||
| Thread.sleep(50L); | ||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even so, how would retrying help in this situation?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the wait time, the other process or thread will have released its lock. Bear in mind the os is doing so to prevent deadlock.
There is similar code in the Android https://github.com/androidx/androidx/blob/1c55c017b56b3d702e8013a0aa9baee866ea9fb7/datastore/datastore-core/src/androidMain/kotlin/androidx/datastore/core/MultiProcessCoordinator.android.kt#L77
and coursier
https://github.com/coursier/coursier/blob/345d58581abafac56891dcbaef24ec6e79ce0fb5/modules/bootstrap-launcher/src/main/java/coursier/bootstrap/launcher/Download.java#L208
as well as the comment added here.
Maybe we should limit to this
if(e.getMessage().contains("Resource deadlock avoided"))though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC this can only happen if the other process owning the lock is somehow waiting for this process. Otherwise why would it otherwise be detected as DEADLOCK situation in the first place? With the 2nd attempt how will the owning process be unblocked. IMHO retry only makes sense if some lock is released in between (the other process is waiting for). How would that be the case here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read the message "Resource deadlock avoided" which means the deadlock has not happened yet
And this the key point, EDEADLK prevents the deadlock rather than reporting one that already exists.
When the kernel returns EDEADLK, it means he refused to grant the lock and returns immediately, the calling process is NOT blocked. The other process (which is blocked waiting) can now proceed because the cycle no longer exists. It completes its work, releases its locks, and when we retry after the 50ms sleep, the contention is resolved.
It looks this deadlock detection is known to eventually produce false positives [1] as it's a bit conservative. So the retry simply succeeds immediately.
Maybe adding
e.getMessage().contains("Resource deadlock avoided")would be more precise, so we don't accidentally swallow unrelated IOExceptions during the lock attempt. I can add that filter.Here the sequence:
[1] https://man7.org/linux/man-pages/man2/F_SETLK.2const.html section Bugs Deadlock detection