Make Retriable config thread-safe (copy-on-write) - #151
Open
kamui wants to merge 11 commits into
Open
Conversation
Closed
kamui
force-pushed
the
thread-safe-config
branch
from
June 13, 2026 20:00
f5da97a to
355429c
Compare
Config#dup now deep-copies its mutable members (on, intervals, contexts) via initialize_copy so a duplicated config is fully isolated from the original. Scalars, procs and exception classes stay shared by reference. This is the foundation for a copy-on-write Retriable.configure.
Closes two thread-safety bugs: - Init race: @config ||= Config.new was a check-then-act race that could create two Config objects under concurrent boot. @config is now eagerly initialized at load time (require is serialized in MRI). - Torn reads: retriable's fast path read attributes off the live shared config one at a time, so a concurrent configure could yield an inconsistent mix. configure is now copy-on-write under CONFIG_MUTEX (dup -> yield -> atomic publish), so readers always observe a consistent, never-mutated snapshot. configure still does not validate; validation stays lazy at retriable-call time.
with_context previously read the published config multiple times (the existence check via available_contexts and option resolution via context_options_for, and config_contexts itself read config twice). A concurrent configure publishing a new config between those reads could let the existence check pass on the old snapshot while options resolved against the new one, silently dropping the context's retry options. Capture one config snapshot at the top of with_context and thread it through available_contexts, context_options_for, and config_contexts so the whole context resolution sees a single, consistent config.
with_context captured a config snapshot for context lookup and option resolution, but then called retriable, which re-read config.to_h. A concurrent configure between the snapshot capture and that re-read could combine the old context's options with new global config values (e.g. a changed on/retry_if list). Extract retriable's body into a private retriable_with_config(base_config, opts) helper that reads everything from base_config. Public retriable delegates with the live config (one read, unchanged behavior); with_context delegates with its captured snapshot, so the whole operation sees one consistent config.
Config#dup only copied the top-level on/intervals/contexts containers and each context's options hash one level deep, so mutable values nested deeper (an intervals array inside a context, or the collection values of a Hash on) stayed shared with the original. An in-place mutation inside a configure block could then corrupt the previously published config. Replace the two shallow helpers with a recursive deep_dup that copies Hash/Array/Set containers at every level and leaves scalars, procs, exception classes, and regexps shared by reference.
Address two PR review findings on the copy-on-write config:
- Guard #configure against reentrancy: nesting now raises an explicit
ThreadError ("cannot be nested") via CONFIG_MUTEX.owned? instead of a
cryptic recursive-locking deadlock. Preserves the loud-failure semantics
(a nested update can't silently clobber the outer publish).
- Make the @config read correct on non-MRI engines. The lock-free read
relies on MRI's GVL for a happens-before edge that JRuby/TruffleRuby do
not provide. Gate it on RUBY_ENGINE == "ruby" (LOCK_FREE_CONFIG_READ);
other engines read through CONFIG_MUTEX so they synchronize with
#configure's publishing write. The JRuby CI job exercises this path.
kamui
force-pushed
the
thread-safe-config
branch
from
July 13, 2026 04:24
8610bc5 to
f3eab66
Compare
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
1 participant
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
configureretriable,with_context, and fiberswith_contextagainst one snapshotconfigurecalls, including calls from another fiber in the configuring thread, with an actionableThreadErrorThread-safety contract
configurecalls are serialized and publish only after a successful blockRetriable.configremains outside the thread-safety guarantee; callers should useRetriable.configureVerification
bundle exec rspec(190 examples, 0 failures; 99.6% line coverage)bundle exec rubocop(15 files, no offenses)bundle exec rbs -I sig validate