Skip to content

Fix concurrent Unix named mutex creation - #134111

Merged
jkoritzinsky merged 10 commits into
mainfrom
dev/jkoritzinsky/shiny-adventure
Sep 24, 2026
Merged

jkoritzinsky merged 10 commits into
mainfrom
dev/jkoritzinsky/shiny-adventure

Conversation

@jkoritzinsky

Copy link
Copy Markdown
Member

Why

Concurrent shared compilations on Linux can intermittently fail when a newly created named mutex is initially owned. The creator can lose the mutex between publishing the shared state and recording ownership, causing a later ReleaseMutex to fail with "Cannot release a lock that is not owned by the current thread."

What changed

  • Keep the creation/deletion process lock held through nonblocking initial mutex acquisition and ownership bookkeeping.
  • Preserve the existing behavior of releasing that lock before blocking waits.
  • Add a Unix regression test covering concurrent same-name constructors with initial ownership.

Testing

  • ./build.sh clr+libs -rc checked
  • ./build.sh clr.corelib+clr.nativecorelib+libs.pretest -rc checked
  • dotnet test src/libraries/System.Threading/tests/System.Threading.Tests.csproj --no-build (714 passed)
  • Focused regression test passed.

Resolves #134043

Note

This pull request description was generated with GitHub Copilot assistance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @VSadov
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Add platform capability guards to prevent unsupported targets from failing or hanging.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

Fixes a Unix named-mutex creation race by preserving the process lock through initial acquisition and ownership bookkeeping, with a concurrent regression test.

Changes:

  • Updates Unix mutex acquisition locking.
  • Adds concurrent initial-ownership coverage.
  • Test requires platform capability guards for unsupported targets.
File Description
src/​libraries/​System.Threading/​tests/​MutexTests.cs Adds concurrent named-mutex constructor coverage; requires guards for mobile, Browser, and Wasi.
src/​libraries/​System.Private.CoreLib/​src/​System/​Threading/​NamedMutex.Unix.cs Preserves locking during nonblocking ownership setup.

Comment thread src/libraries/System.Threading/tests/MutexTests.cs Outdated
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 17, 2026 02:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

Resolved since last review (1)

@jkoritzinsky

Copy link
Copy Markdown
Member Author

/ba-g JSON oom unrelated

Comment thread src/libraries/System.Threading/tests/MutexTests.cs Outdated
Comment thread src/libraries/System.Private.CoreLib/src/System/Threading/NamedMutex.Unix.cs Outdated
…es the mutex. Preserve the assert as an exception-throw in release builds instead of a debug-only assert.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Add actionable context to the new invariant exception before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Low severity

Open (1)

Comment thread src/libraries/System.Private.CoreLib/src/System/Threading/NamedMutex.Unix.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Unresolved mutex acquisition and regression-test issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Avoid process lock for timed-out or recursive waits

src/​libraries/​System.Private.CoreLib/​src/​System/​Threading/​NamedMutex.Unix.cs:672

This acquires the global creation/deletion process lock for every result, even though RecordMutexAcquisition immediately returns for TimedOut and AcquiredLockRecursively. A timed-out or recursive wait can therefore block behind unrelated named-mutex creation/deletion work despite doing no bookkeeping; only acquire the process lock when the raw result needs ownership recording.

Comment thread src/libraries/System.Private.CoreLib/src/System/Threading/NamedMutex.Unix.cs Outdated
Copilot AI review requested due to automatic review settings September 23, 2026 21:50
@jkoritzinsky
jkoritzinsky force-pushed the dev/jkoritzinsky/shiny-adventure branch from 29f591c to d8d7a8b Compare September 23, 2026 21:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

A moderate issue affects the nonblocking behavior of zero-timeout waits, and a minor unused local remains.

Review effort: Lite
Findings: None

Resolved since last review (1)

@agocke

agocke commented Sep 23, 2026

Copy link
Copy Markdown
Member

Can you file a bug for the mono issue? In case we need to know what the problem is in the future?

@jkoritzinsky

Copy link
Copy Markdown
Member Author

I think I may have addressed the Mono issue with the other fix. I'll revert the Mono disable and have CI check it out for me.

Copilot AI review requested due to automatic review settings September 23, 2026 22:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Timed and nonblocking acquisition must avoid blocking on the creation lock.

Review effort: Lite
Findings: None

@jkoritzinsky

Copy link
Copy Markdown
Member Author

Yep the runs on Mono caught the bug because the Thread.Join implementation there is less robust.

We're good to go.

@jkoritzinsky

Copy link
Copy Markdown
Member Author

/ba-g ssl stream tests unrelated

@jkoritzinsky

Copy link
Copy Markdown
Member Author

/backport to release/11.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/11.0 (link to workflow run)

@jkoritzinsky
jkoritzinsky merged commit 9a4b4fc into main Sep 24, 2026
141 of 143 checks passed
@jkoritzinsky
jkoritzinsky deleted the dev/jkoritzinsky/shiny-adventure branch September 24, 2026 16:41
@github-project-automation github-project-automation Bot moved this to Done in AppModel Sep 24, 2026
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Sep 25, 2026
JulieLeeMSFT pushed a commit that referenced this pull request Sep 25, 2026
Backport of #134111 to release/11.0

/cc @jkoritzinsky

## Customer Impact

- [x] Customer reported
- [ ] Found internally

Creating a new named mutex with the `initiallyOwned` parameter set to
`true` from two threads in the same process could allow one of the
threads to proceed without owning the mutex on Unix platforms.

This was found by Roslyn in their move to use multithreaded MSBuild
tasks.

## Regression

- [X] Yes
- [ ] No

#117635

## Testing

Unit test added that forces this particular race many times in a loop
until it occurs.

## Risk

Low, only touches named mutex logic and the test validates the exact
failure case reliably (always hits in the first 150 iterations on
failure).

Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Co-authored-by: Jeremy Koritzinsky <Jeremy.Koritzinsky@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Concurrent shared compilations can fail releasing the client mutex on Linux

5 participants