Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add a simple deadlock detector for require locks.
Browse files Browse the repository at this point in the history
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 committed Aug 20, 2016
1 parent fc2dd1e commit fe83dfc
Showing 1 changed file with 58 additions and 11 deletions.
69 changes: 58 additions & 11 deletions core/src/main/java/org/jruby/runtime/load/LoadService.java
Original file line number Diff line number Diff line change
@@ -36,6 +36,9 @@
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.lang.management.ManagementFactory;
import java.lang.management.ThreadInfo;
import java.lang.management.ThreadMXBean;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URI;
@@ -44,7 +47,9 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.ReentrantLock;
import java.util.jar.JarFile;
@@ -197,6 +202,7 @@ public LoadService(Ruby runtime) {
}

this.librarySearcher = new LibrarySearcher(this);
this.requireLocks = new RequireLocks(runtime);
}

/**
@@ -404,18 +410,32 @@ private enum RequireState {
LOADED, ALREADY_LOADED, CIRCULAR
}

private final RequireLocks requireLocks = new RequireLocks();
private final RequireLocks requireLocks;

private static final class RequireLocks {
private final ConcurrentHashMap<String, ReentrantLock> pool;
// global lock for require must be fair
//private final ReentrantLock globalLock;
private final ConcurrentHashMap<String, RequireLock> pool = new ConcurrentHashMap<>(8, 0.75f, 2);
private final Ruby runtime;

public enum LockResult { LOCKED, CIRCULAR }

private RequireLocks() {
this.pool = new ConcurrentHashMap<>(8, 0.75f, 2);
//this.globalLock = new ReentrantLock(true);
private class RequireLock extends ReentrantLock {
private final String file;

public RequireLock(String file) {
this.file = file;
}

public Thread getOwner() {
return super.getOwner();
}

public String getFile() {
return file;
}
}

private RequireLocks(Ruby runtime) {
this.runtime = runtime;
}

/**
@@ -429,21 +449,48 @@ private RequireLocks() {
* returns false without getting a lock. Otherwise true.
*/
private LockResult lock(String requireName) {
ReentrantLock lock = pool.get(requireName);
RequireLock lock = pool.get(requireName);

if (lock == null) {
ReentrantLock newLock = new ReentrantLock();
RequireLock newLock = new RequireLock(requireName);
lock = pool.putIfAbsent(requireName, newLock);
if (lock == null) lock = newLock;
}

if (lock.isHeldByCurrentThread()) return LockResult.CIRCULAR;

lock.lock();
return lockWithDeadlockDetection(lock);
}

return LockResult.LOCKED;
private LockResult lockWithDeadlockDetection(RequireLock lock) {
while (true) {
Thread owner = lock.getOwner();
if (owner != null) {
// already locked, scan for deadlocks
ThreadMXBean tmxb = ManagementFactory.getThreadMXBean();
ThreadInfo ownerInfo = tmxb.getThreadInfo(new long[] {owner.getId()}, false, true)[0];

if (ownerInfo != null && ownerInfo.getLockOwnerId() == Thread.currentThread().getId()) {
// deadlock detected; owner thread is waiting on a lock we own
throw runtime.newLoadError("threads \"" + owner.getName() + "\" and \"" + Thread.currentThread().getName() + "\" will deadlock requiring \"" + lock.file + "\"");
}
}

// Otherwise try to lock for a variable amount of time and check again.
// We use a variable amount of time to decrease the likelihood that the deadlocking threads will
// always appear to be running and not waiting on a lock.
try {
boolean locked = lock.tryLock(500 + (int)(random.nextDouble() * 100), TimeUnit.MILLISECONDS);

if (locked) return LockResult.LOCKED;
} catch (InterruptedException ie) {
// ignore, proceed back to deadlock check
}
}
}

private static final Random random = new Random(System.currentTimeMillis());

/**
* Unlock the lock for the specified requireName.
*

0 comments on commit fe83dfc

Please sign in to comment.