Repository navigation
Make the remaining fiber-visible thread-locals fiber-safe (#578) - #579
Merged
Merged
Conversation
The v0.1.0 image segfaulted on linux/arm64 right after the first write through a POV. The faulting PC is TLocalReadFileCache::Get with a null this, called from the TSafeRepo::RemoveFile lambda that visits every scheduler to clear its read-file cache. That lambda reads tpidr_el0 once at entry, SwitchTo()s onto the target scheduler's thread, then derives Cache's address from the stale thread pointer, so it reads the calling thread's slot, which is null there. It is the #554 hazard, on one of the three thread-locals #555 left unaudited. Convert all three to TFiberSafeLocal: TLocalReadFileCache::Cache, TLocalWalkerCache::Cache and TDiskController::TEvent::LocalEventPool. EnsureLocalDiskEventPool bound a reference to the slot, which caches the thread pointer just the same, so it now goes through the accessor. Verified on native aarch64 (release, LTO): the lambda no longer has an mrs, and calls the accessor after switch_to_fiber. A loop that writes through safe shared, fast shared and safe private POVs killed the unfixed build 10/10 times (5/5 with freshly compiled packages) and the fixed build 0/20.
This was referenced Oct 2, 2026
Merged
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.
Fixes #578.
Cause
The kernel's fault report for the
v0.1.0image puts the PC atorlyi+0x613ff4:TLocalReadFileCache<...>::Get, faulting on its first load fromthis, withthis == nullptr. Its caller is the lambda inTSafeRepo::RemoveFilethat visits each scheduler to clear its read-file cache:In the shipped binary that lambda does
mrs x27, tpidr_el0at entry (0x6144c8), callsswitch_to_fiber(0x614524), and then derivesCache's address from the stalex27(0x61452c). After the switch the fiber is running on the target scheduler's thread, but it reads the calling thread's slot, which is null there. This is the #554 hazard on one of the three thread-locals that #555 deliberately left unaudited. amd64 is unaffected because x86-64 folds the TLS base into each access.Fix
The three remaining fiber-visible thread-locals become
TFiberSafeLocal, the same asTRunner::LocalRunnerand friends:TLocalReadFileCache::CacheTLocalWalkerCache::Cache(read in the same lambda)TDiskController::TEvent::LocalEventPoolTJumpRunnable::EnsureLocalDiskEventPoolbound apool_t *&to the slot. A reference caches the thread pointer the same way, so it now goes through the accessor. Call sites are otherwise unchanged.TReadFile::HashHitCountis still a bare__thread. It's a statistics counter, so a wrong-thread increment there miscounts but can't crash.Verification
Native aarch64 (Docker on Apple Silicon), release build with LTO:
RemoveFilelambda has nomrs, and it callsTFiberSafeLocal<...>::Get()afterswitch_to_fiber. Current master still hoists themrs.orlyicommand line, writessample.write_valthrough asafe shared, afast sharedand asafe privatePOV, then checks the server is still alive.CI's aarch64 job builds debug, which doesn't hoist, so it can't catch this class. That gap is #556.