Fix recursive-mutex deadlock in Library#sync_catalog - #1220
Merged
castwide merged 1 commit intoAug 3, 2026
Merged
Conversation
Library#cache_next_gemspec's "already being processed" branch called sync_catalog to try the next cacheable gemspec, but that method is only ever reached while already inside sync_catalog's own mutex.synchronize block (directly, or via this same recursive path), so re-entering it re-locks a non-reentrant Thread::Mutex the current thread already holds, raising ThreadError: deadlock; recursive locking. Recurse into cache_next_gemspec directly instead, since the mutex is already held for the whole call chain. Fixes castwide#1111
apiology
marked this pull request as ready for review
July 29, 2026 12:31
apiology
force-pushed
the
fix-sync-catalog-recursive-mutex-deadlock
branch
from
July 31, 2026 11:49
fd3a519 to
fc77012
Compare
apiology
marked this pull request as draft
August 2, 2026 14:43
apiology
marked this pull request as ready for review
August 2, 2026 17:19
Contributor
Author
|
@castwide Ready for review |
This was referenced Aug 2, 2026
Owner
|
Tested locally, looks good 👍 |
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
2 participants
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.
Summary
Fixes #1111 (
ThreadError: deadlock; recursive locking).Library#cache_next_gemspec's "already being processed" branch enqueuesthe gemspec and, if other gemspecs are still pending, recurses to try
the next one. It did this by calling
sync_catalogagain — butcache_next_gemspecis only ever reached from insidesync_catalog'sown
mutex.synchronizeblock (either directly, or transitively viathis same recursive path on the same thread), so re-entering
sync_catalogtries to re-lock a non-reentrantThread::Mutexthecurrent thread already holds, raising the deadlock error from #1111's
stack trace exactly.
The fix recurses into
cache_next_gemspecdirectly instead of throughsync_catalog, since the mutex is already held for the entire callchain by the time this branch runs.
This bug predates the pin-caching rework and its revert (#1180) — it
was introduced in #990 and is independent of that whole saga, but it's
one of the "deadlocked threads" issues mentioned in #1180's PR
description as motivation for reverting, so I wanted to get it fixed
and out of the way on its own before looking at anything else in that
area.
Test plan
spec/library_spec.rb) that stubsYardoc.processing?to force the recursive branch with multiplepending gemspecs; it reproduces the exact deadlock on current
master and passes after the fix.
bundle exec rspec— full suite green (2 pre-existing, unrelatedfailures in
rbs_map/conversions_spec.rb/activesupport_concern_spec.rbreproduce identically on master without this change).
bundle exec rubocopclean on changed files.