Skip to content
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

Add a simple deadlock detector for require locks. #4095

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

headius
Copy link
Member

@headius headius commented Aug 20, 2016

This detector works only with two threads, using the following
heuristic:

  1. If the lock is already acquired, obtain its owner thread.
  2. Check if the owner thread is waiting on a lock.
  3. If the lock the owner is waiting on is held by the current
    thread, raise an error.
  4. If there's no owner and no deadlock, try to lock the thread for
    500ms + rand(100)ms.
  5. If the file has been locked, return success.
  6. Otherwise, start over at 1.

We may want a more advanced detector that can handle arbitrarily
many threads, but it would be much heavier and require scanning
all threads in the system, including those unrelated to the
current program and those not actually trying to require files.

See #3341.

@headius
Copy link
Member Author

headius commented Aug 20, 2016

I'm opening this PR to discuss the implementation and seek input.

A few known issues:

  • Hard dependency on management extensions. These are not available on all platforms and I have no fallbacks (nor graceful handling when those extensions are unavailable).
  • Potentially heavy to acquire this info. Does it matter if we only do it for contended threads?
  • Only works for two threads with mutually-exclusive locking. There are algorithms for detecting deadlocks between many threads, but I believe they're NP. I have not researched yet.

@headius
Copy link
Member Author

headius commented Aug 20, 2016

I made a modification to mitigate known issue 2 above: it now will attempt to lock FIRST, only doing deadlock detection if that attempt fails.

@kares
Copy link
Member

kares commented Aug 20, 2016

should safe fail when mx (java.lang.management) not available e.g. on Android

@headius
Copy link
Member Author

headius commented Aug 22, 2016

@kares Indeed...it would likely just skip this code.

This general pattern may be applicable elsewhere, so this should move into its own utility class.

@headius
Copy link
Member Author

headius commented Aug 22, 2016

Punting to 9.1.4.0. Too risky to stitch in at the last moment for 9.1.3.0.

@headius
Copy link
Member Author

headius commented Nov 8, 2016

Needs rebase and more thought. Bumping and rebasing.

This detector works only with two threads, using the following
heuristic:

1. If the lock is already acquired, obtain its owner thread.
2. Check if the owner thread is waiting on a lock.
3. If the lock the owner is waiting on is held by the current
   thread, raise an error.
4. If there's no owner and no deadlock, try to lock the thread for
   500ms + rand(100)ms.
5. If the file has been locked, return success.
6. Otherwise, start over at 1.

We may want a more advanced detector that can handle arbitrarily
many threads, but it would be much heavier and require scanning
all threads in the system, including those unrelated to the
current program and those not actually trying to require files.

See #3341.
@headius headius added this to the JRuby 9.2.0.0 milestone Dec 14, 2016
@headius headius removed this from the JRuby 9.1.7.0 milestone Dec 14, 2016
@enebo enebo modified the milestones: JRuby 9.2.0.0, JRuby 9.2.1.0 May 24, 2018
@headius
Copy link
Member Author

headius commented Oct 11, 2018

De-targeting. This may be useful some day but there's work required and many open questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants