Skip to content

fix!(core): Make HubSwitchGuard !Send to prevent thread corruption#957

Draft
szokeasaurusrex wants to merge 4 commits intomasterfrom
szokeasaurusrex/hubswitchguard
Draft

fix!(core): Make HubSwitchGuard !Send to prevent thread corruption#957
szokeasaurusrex wants to merge 4 commits intomasterfrom
szokeasaurusrex/hubswitchguard

Conversation

@szokeasaurusrex
Copy link
Member

@szokeasaurusrex szokeasaurusrex commented Jan 14, 2026

Description

HubSwitchGuard manages thread-local hub state but was Send, allowing it to be moved to another thread. When dropped on the wrong thread, it could corrupt that thread's hub state instead of restoring the original thread.

This PR makes HubSwitchGuard !Send by adding PhantomData<MutexGuard<'static, ()>> while keeping it Sync. The type system now prevents the guard from being moved across threads at compile time.

To ensure guards are always dropped on the originating thread, sentry-tracing now stores them in thread-local storage keyed by span ID rather than in span extensions.

Issues

@linear
Copy link

linear bot commented Jan 14, 2026

@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/hubswitchguard branch from 21b407b to 1dea844 Compare January 14, 2026 12:28
@szokeasaurusrex szokeasaurusrex changed the title fix(core): Make HubSwitchGuard !Send to prevent thread corruption fix!(core): Make HubSwitchGuard !Send to prevent thread corruption Jan 14, 2026
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/hubswitchguard branch from e78483a to 8abf004 Compare January 14, 2026 12:54
@szokeasaurusrex szokeasaurusrex requested a review from lcian January 26, 2026 16:19
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/hubswitchguard branch from 8abf004 to 1255c8a Compare January 28, 2026 12:36
@szokeasaurusrex szokeasaurusrex marked this pull request as ready for review January 28, 2026 14:23
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

Copy link
Member

@lcian lcian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

szokeasaurusrex and others added 4 commits February 4, 2026 13:55
HubSwitchGuard manages thread-local hub state but was Send, allowing it
to be moved to another thread. When dropped on the wrong thread, it would
corrupt that thread's hub state instead of restoring the original thread.

To fix this, add PhantomData<MutexGuard<'static, ()>> to make the guard
!Send while keeping it Sync. This prevents the guard from being moved
across threads at compile time.

Additionally, refactor sentry-tracing to store guards in thread-local
storage keyed by span ID instead of in span extensions. This fixes a
related bug where multiple threads entering the same span would clobber
each other's guards.

Fixes #943
Refs RUST-130

Co-Authored-By: Claude <[email protected]>
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/hubswitchguard branch from 0976b79 to e4ddd01 Compare February 4, 2026 12:57
@szokeasaurusrex szokeasaurusrex marked this pull request as draft February 4, 2026 15:34
@szokeasaurusrex
Copy link
Member Author

This current implementation is still flawed; if the same span is re-entered in the same thread (possible in async contexts), the second entry overwrites the original HubSwitchGuard, corrupting the behavior. I am working on a fix

@lcian
Copy link
Member

lcian commented Feb 4, 2026

@szokeasaurusrex it rewrites the original guard with the same one I think, so what's the issue there?

@lcian
Copy link
Member

lcian commented Feb 4, 2026

I've tested some scenarios manually and didn't find any issues. I assume you're also testing manully.
Would be nice to add these as automated tests as well.

@szokeasaurusrex
Copy link
Member Author

szokeasaurusrex commented Feb 4, 2026

@lcian Codex AI agent identified the potential issue here as I was working on #946. I have a test locally which reproduces the behavior. Will commit it with a more detailed explanation of the problem tomorrow 👍

I suppose it's possible that Codex is wrong and the local test is doing something which is not supposed to ever happen (the scenario is admittedly a bit contrived), but in any case, I think it's worth it to investigate properly. So, that is what I'm doing now. I will let you know if I need any assistance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HubSwitchGuard should not be Send

2 participants