Repository navigation
Refactor scoping and make default_scopes thread-safe (rebased) - #772
Merged
Merged
Conversation
We've used `active_scopes` and `default_scopes` to store scopes in the current
context. However, it has been thread-unsafe because we clear `default_scopes`
to unscope.
Moreover, `default_scopes` is the master data of default scopes, and we shouldn't
clear it.
Therefore, this commit introduces `current_context` as a thread local variable to
store the current scoped context in it:
- When `unscoped { ... }` is called, `current_context` stores a plain query object
- When `with_scope(...) { ... }` is called, `current_context` stores a query object
scoped with `default_scopes` and the method's arguments
- Otherwise, it stores nothing - nil, so we need to build a new query with
`default_scopes`
`current_context` includes active scopes naturally, so this commit removes the method.
Inside with_scope and unscoped, query now clones the thread-local context and amend()s criteria plus query options such as limit and sort. where() was folding those options into the criteria hash. A default scope that calls back into query no longer reapplies default scopes on that call, so it does not recurse. The guard is thread-local and does not clear the shared default_scopes array. query also dupes its options hash before removing the internal :unscoped flag. Co-authored-by: Scott Taylor <smtlaissezfaire@users.noreply.github.com>
Bring the thread-safe scope rewrite up to date with current master, including the embedded NoRootDocument change and MongoDB 9.0 CI. Co-authored-by: Scott Taylor <smtlaissezfaire@users.noreply.github.com>
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
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.
Rebase of #683 onto current
master.Originally opened by @a2ikm. Their commit is preserved (author Masato Ikeda); this branch does not modify the original pull request.
lib/mongo_mapper/plugins/scopes.rbhad not changed since 2021, so the commit cherry-picked cleanly. The follow-up commit reconciles the new thread-localcurrent_contextwith howquerybehaves on master.What the original change does
Scopes used to unscope by clearing the shared
@default_scopesarray (and the thread-localactive_scopeslist). Another thread could observe an empty default scope whileunscopedwas running.default_scopesis the stored definition of those scopes and should not be cleared.current_contextis a thread-local query instead:unscoped { ... }stores a plain query, so default scopes are not applied inside the blockwith_scope(...) { ... }stores a query that already includes default scopes plus the arguments, and nestedwith_scopestacks on that queryquerybuilds a new query and appliesdefault_scopesactive_scopesis removed.current_contextis the active scope. Anything callingModel.active_scopeswill break.Rework
Two master behaviors did not survive the first version of
queryoncecurrent_contextwas set:Model.query(limit: 1, sort: :age)insidewith_scope/unscopedwent throughwhere, which puts every key in the criteria hash.limitandsortthen stop being query options.querynowclone.amends the context, which is whatQuerying#querydoes for a fresh query. A later query in the same scope does not keep that limit.whereon the model used to run insideunscoped, which cleared@default_scopesso the innerquerydid not apply default scopes again. Without that, the proc recurses until the stack overflows. The re-entry guard is a thread-local flag. It does not clear@default_scopes, so the thread-safety fix stays intact.queryalso dupes its options hash before deleting the internal:unscopedkey, so the caller's hash is left alone (and a frozen hash does not raise).Risks
active_scopesis gone. It was public enough to have a thread-safety changelog entry. Callers should usewith_scope/unscopedrather than reading the list.mongo_mapper_#{name}_current_context). Two classes that share a name, includingDoc()helpers that both report"Class", still share a context on one thread. That matches the oldactive_scopeskey.process_scopereturns a non-Hash scope as-is. That is unchanged from master. Block-formdefault_scope { where(...) }stores the query produced at definition time, and later default scopes are built on top of earlier ones, which is why stacked default scopes still work._typecriteria are applied when the context query is first built (with_scope,unscoped, or a normalquery). Later queries inside that context clone it, so_typeis kept. Identity-map marking still happens on the queryIdentityMap#queryreturns.