FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add a simple deadlock detector for require locks. by headius · Pull Request #4095 · jruby/jruby · GitHub

/ jruby Public

Add a simple deadlock detector for require locks. - #4095

Open
headius wants to merge 3 commits into
masterfrom
require_deadlock
Open

Add a simple deadlock detector for require locks.#4095
headius wants to merge 3 commits into
masterfrom
require_deadlock

Conversation

headius commented Aug 20, 2016

Copy link
Copy Markdown
Member

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 commented Aug 20, 2016

Copy link
Copy Markdown
Member Author

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 commented Aug 20, 2016
edited
Loading

Copy link
Copy Markdown
Member Author

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 commented Aug 20, 2016

Copy link
Copy Markdown
Member

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

headius commented Aug 22, 2016

Copy link
Copy Markdown
Member Author

@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 commented Aug 22, 2016

Copy link
Copy Markdown
Member Author

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

headius commented Nov 8, 2016

Copy link
Copy Markdown
Member Author

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 added this to the JRuby 9.2.0.0 milestone Dec 14, 2016
headius removed this from the JRuby 9.1.7.0 milestone Dec 14, 2016
enebo modified the milestones: JRuby 9.2.0.0, JRuby 9.2.1.0 May 24, 2018

headius commented Oct 11, 2018

Copy link
Copy Markdown
Member Author

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL