Recover from AbandonedMutexException in GlobalMutex (Windows)#1504
Closed
myieye wants to merge 1 commit into
Closed
Recover from AbandonedMutexException in GlobalMutex (Windows)#1504myieye wants to merge 1 commit into
myieye wants to merge 1 commit into
Conversation
Per the .NET Mutex contract, AbandonedMutexException means the wait succeeded and the calling thread now owns the mutex; the exception is purely informational. The previous behaviour propagated it, crashing every libpalaso consumer that takes the lock via `using (mutex.Lock())`. Trace the recovery so self-abandonment by a thread in our own process (a real bug, distinct from the cross-process case this catch absorbs) stays visible. Fixes FieldWorks LT-21834; also covers a recently reported shutdown-time crash from the WritingSystemManager.Save path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Acquire and intentionally don't release: thread exit marks the mutex abandoned. | ||
| var worker = new Thread(() => | ||
| { | ||
| var raw = new Mutex(false, name); |
Collaborator
Author
|
This is obviously somewhat hairy. I'm not actually up for this right now. |
Contributor
|
Should an issue be opened instead? |
Collaborator
Author
|
I added a comment to the Jira issue pointing at the PR. |
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
3 participants
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.
A bit of a gutsy PR perhaps.
Fixes LT-21834 and a related shutdown-time crash from
WritingSystemManager.Save→GlobalWritingSystemRepository.TryGet→GlobalMutex.Lock().WindowsGlobalMutexAdapter.Wait()now catchesAbandonedMutexException, traces a warning, and proceeds. Why this feels like a safe change:AbandonedMutexExceptiondocs: "Whether or not the exception was thrown, the current thread owns the mutex, and must release it." Subsequent_mutex.ReleaseMutex()inRelease()succeeds.AbandonedMutexExceptioncarries no information about who abandoned the mutex or why — propagating it just produced an unhandled crash. TheTrace.TraceWarningpreserves the only signal that actually exists ("an abandonment was observed here").GlobalMutex.Lock()(the new shutdown crash) andGlobalMutex.InitializeAndLock()→Init(initiallyOwned: true)(LT-21834) funnel throughWait(). TheInitstack shows the call frame becauseWait()is JIT-inlined; thetry/catchis preserved through inlining.As far as I know/understand, the one real downside of this fix is that we/users will stop seeing crashes and errors and although those errors will likely not be useful for pinpointing bugs, they do indicate that there is presumably code out there somewhere that is not handling the mutex correctly.
This change is