Skip to content
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

Jump to bottom

core: fix scoped dispatchers clobbering the global default #2065

Merged
merged 2 commits into from
Apr 12, 2022
Merged

core: fix scoped dispatchers clobbering the global default #2065

merged 2 commits into from
Apr 12, 2022

Commits on Apr 11, 2022

  1. add a test reproducing #2050

    @hawkw
    hawkw committed Apr 11, 2022
    Configuration menu
    Copy the full SHA
    b73d396 View commit details
    Browse the repository at this point in the history

  2. core: fix scoped dispatchers clobbering the global default 

    ## Motivation

PR #2001 introduced --- or rather, _uncovered_ --- a bug which occurs
when a global default subscriber is set *after* a scoped default has
been set.

When the scoped default guard is dropped, it resets the
thread-local default cell to whatever subscriber was the default when
the scoped default was set. This allows nesting scoped default contexts.
However, when there was *no* default subscriber when the `DefaultGuard`
was created, it sets the "previous" subscriber as `NoSubscriber`. This
means dropping a `DefaultGuard` that was created before any other
subscriber was set as default will reset that thread's default to
`NoSubscriber`. Because #2001 changed the dispatcher module to stop
using `NoSubscriber` as a placeholder for "use the global default if one
exists", this means that the global default is permanently clobbered on
the thread that set the scoped default prior to setting the global one.

## Solution

This PR changes the behavior when creating a `DefaultGuard` when no
default has been set. Instead of populating the "previous" dispatcher
with `NoSubscriber`, it instead leaves the `DefaultGuard` with a `None`.
When the `DefaultGuard` is dropped, if the subscriber is `None`, it will
just clear the thread-local cell, rather than setting it to
`NoSubscriber`. This way, the next time the cell is accessed, we will
check if a global default exists to populate the thread-local, and
everything works correctly. As a side benefit, this also makes the code
a bit simpler!

I've also added a test reproducing the bug.

This PR is against `v0.1.x` rather than `master`, because the issue does
not exist on `master` due to other implementation differences in v0.2.
We may want to forward-port the test to guard against future
regressions, though.

Fixes #2050
    @hawkw
    hawkw committed Apr 11, 2022
    Configuration menu
    Copy the full SHA
    64d0e19 View commit details
    Browse the repository at this point in the history