Skip to content

Replace CheapUnfairReaderWriterLock with ReaderWriterLockSlim - #136241

Merged
jkotas merged 6 commits into
dotnet:mainfrom
asylkhan-azat:transactions-lock-ordering-bug
Oct 22, 2026
Merged

jkotas merged 6 commits into
dotnet:mainfrom
asylkhan-azat:transactions-lock-ordering-bug

Conversation

@asylkhan-azat

@asylkhan-azat asylkhan-azat commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Replace CheapUnfairReaderWriterLock with ReaderWriterLockSlim.

Generate transaction hash in table via counter.

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 21, 2026
@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: @SamMonoRT, @dotnet/efteam
See info in area-owners.md if you want to be subscribed.

@asylkhan-azat

asylkhan-azat commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor Author

Oops, didn't know that MRES isn't supported on brower. Reverted back to use MRE

@asylkhan-azat
asylkhan-azat force-pushed the transactions-lock-ordering-bug branch from 83f69f0 to 2651cf2 Compare October 22, 2026 17:27
Generate transaction hashes in table itself
@jkotas

jkotas commented Oct 22, 2026

Copy link
Copy Markdown
Member

Could you please update the PR description to match the final change?

@asylkhan-azat

Copy link
Copy Markdown
Contributor Author

@jkotas Sure, done

Could you please update the PR description to match the final change?

@asylkhan-azat asylkhan-azat changed the title Fix memory ordering in CheapUnfairReaderWriterLock Replace CheapUnfairReaderWriterLock with ReaderWriterLockSlim Oct 22, 2026
@jkotas
jkotas requested a balanced review from Copilot October 22, 2026 22:49

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.

🟡 Changes recommended

The replacement lock rejects nested transaction creation from synchronous timer listeners, and concurrency validation remains outstanding.

1 open issue
What changed in this PR

This PR replaces the custom transaction-table lock with ReaderWriterLockSlim and separates transaction hash generation from lock bookkeeping.

Changes:

  • Remove CheapUnfairReaderWriterLock and use the standard reader/writer lock.
  • Generate transaction hashes using a dedicated counter incremented with Interlocked.

Validation was limited to source inspection; no builds or tests were run.

File Description
src/​libraries/​System.Transactions.Local/​src/​System/​Transactions/​TransactionTable.cs Replace the custom lock and introduce independent transaction hash generation.

Review effort: Balanced

@jkotas
jkotas merged commit f58b953 into dotnet:main Oct 22, 2026
90 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Transactions community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants