Skip to content

Always remove the lock from pool on failure #8871

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.

Sign up for GitHub

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
headius merged 1 commit into from
Jun 20, 2025
Merged

Always remove the lock from pool on failure #8871

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 44 additions & 36 deletions core/src/main/java/org/jruby/runtime/load/LoadService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,51 +455,59 @@ private RequireState lock(
ThreadContext currentContext = runtime.getCurrentContext();
RubyThread thread = currentContext.getThread();

RequireLock lock = pool.get(path);

// Check if lock is already there
if (lock == null) {
RequireLock newLock = new RequireLock();
// loop until require fails for us or succeeds for another thread
while (true) {
RequireLock lock = pool.get(path);

// Check if lock is already there
if (lock == null) {
RequireLock newLock = new RequireLock();

// If lock is new, lock and return LOCKED
lock = pool.computeIfAbsent(path, (name) -> {
thread.lock(newLock);
return newLock;
});

RequireState state = null;
if (lock == newLock) {
// Lock is ours, run ifLocked and then clean up
try {
return state = executeAndClearLock(path, ifLocked, thread, lock);
} finally {
// failed load, remove our lock and let other threads fight it out
if (state == null) pool.remove(path);
}
}
}

// If lock is new, lock and return LOCKED
lock = pool.computeIfAbsent(path, (name) -> {
thread.lock(newLock);
return newLock;
});
if (lock.isHeldByCurrentThread()) {
// we hold the lock, which means we're re-locking for the same file; warn about this
if (circularRequireWarning && runtime.isVerbose()) {
warnCircularRequire(path);
}

if (lock == newLock) {
// Lock is ours, run ifLocked and then clean up
return executeAndClearLock(path, ifLocked, thread, lock);
return null;
}
}

if (lock.isHeldByCurrentThread()) {
// we hold the lock, which means we're re-locking for the same file; warn about this
if (circularRequireWarning && runtime.isVerbose()) {
warnCircularRequire(path);
// Other thread holds the lock, wait to acquire
while (true) {
try {
thread.lockInterruptibly(lock);
break;
} catch (InterruptedException ie) {
currentContext.pollThreadEvents();
}
}

return null;
}

// Other thread holds the lock, wait to acquire
while (true) {
try {
thread.lockInterruptibly(lock);
break;
} catch (InterruptedException ie) {
currentContext.pollThreadEvents();
// Lock has been acquired, confirm other thread has completed and return default
thread.unlock(lock);
if (lock.destroyed) {
return defaultResult;
}
}

// Lock has been acquired, confirm other thread has completed and return default
if (lock.destroyed) {
thread.unlock(lock);
return defaultResult;
// Other thread failed to load, try again to lock and load
}

// Other thread failed to load, try on this thread instead
return executeAndClearLock(path, ifLocked, thread, lock);
}

private RequireState executeAndClearLock(String path, Function<String, RequireState> ifLocked, RubyThread thread, RequireLock lock) {
Expand Down