MONGOID-5915 avoid async state corruption by forcing child fibers to copy parent state#6143
MONGOID-5915 avoid async state corruption by forcing child fibers to copy parent state#6143jamis wants to merge 1 commit intomongodb:masterfrom
Conversation
…copy parent state
There was a problem hiding this comment.
Pull request overview
This PR strengthens Mongoid’s fiber isolation mode by preventing sibling fibers from clobbering each other’s Mongoid::Threaded state, while still allowing child fibers to inherit an initial snapshot of parent state.
Changes:
- Add integration specs covering interleaved sibling fibers to ensure state isolation and parent-state preservation.
- Track fiber-local storage ownership and copy inherited storage on first access in child fibers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| spec/integration/isolation_state_spec.rb | Adds sibling-fiber interleaving tests to validate isolation semantics in fiber mode. |
| lib/mongoid/threaded.rb | Implements owner tracking for fiber-local storage and copies inherited state to avoid shared mutable storage across fibers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Fiber[] storage is inherited by child fibers, so multiple sibling | ||
| # fibers would otherwise share a single mutable hash. We detect | ||
| # inheritance by comparing the stored owner ID to the current fiber; | ||
| # on first access by a new fiber we copy the inherited state so each | ||
| # fiber starts with a snapshot of its parent's state rather than a | ||
| # shared reference. | ||
| if Fiber[STORAGE_OWNER_KEY] != Fiber.current.object_id | ||
| Fiber[STORAGE_KEY] = (Fiber[STORAGE_KEY] || {}).dup | ||
| Fiber[STORAGE_OWNER_KEY] = Fiber.current.object_id |
There was a problem hiding this comment.
In fiber isolation mode, duplicating only the top-level storage hash is a shallow copy. Many values stored in Threaded are mutable containers (e.g., stack arrays via stack, autosaves/validations/touch_merged hashes containing arrays, modified_documents hash of sets), so sibling fibers can still share and mutate the same nested objects even after dup, which defeats the isolation goal. Consider performing a deep copy of the storage state on inheritance (at least duplicating Arrays/Hashes/Sets and preserving compare_by_identity/default procs where applicable), or alternatively reconstructing a new storage hash and copying keys with safe duplication for known mutable value types.
|
WOOHOO! |
|
❤️ thanks for noticing, @travisbell! I was going to ping you and ask if you could try this out -- it resolves (for me?) the problem with that reproduction script you provided. We hope to release Mongoid 9.1.0 next week sometime, but I freely acknowledge that our deadlines are not your deadlines. :) |
|
👍🏼 I don't mind testing this out at all. I will and report back. |
MONGOID-5882 added the ability to specify isolation state, so that Mongoid could run under either threads or fibers. However, the fiber isolation mode was insufficient and allowed sibling fibers to clobber shared state. This change still allows fibers to inherit state from their parent, but keeps the shared state more isolated.