Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/Microsoft.VisualStudio.Threading/AsyncReaderWriterLock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,24 @@ protected virtual void OnUpgradeableReadLockReleased()
{
}

/// <summary>
/// Invoked when a nested lock request is detected from a forked execution context
/// of an upgradeable read lock holder — specifically, when the calling thread could hold
/// an active lock (it is not an STA thread) but lacks the required
/// <see cref="NonConcurrentSynchronizationContext"/>.
/// </summary>
/// <remarks>
/// <para>This typically indicates that code holding an upgradeable read lock used
/// <c>ConfigureAwait(false)</c> or otherwise left the <see cref="NonConcurrentSynchronizationContext"/>,
/// then requested a nested read lock on the forked context.</para>
/// <para>This method is called <em>outside</em> the lock's private synchronization object.
/// Implementations should be lightweight (e.g., setting a flag or posting a telemetry event).</para>
/// <para>The default implementation does nothing. Override to log diagnostics or telemetry.</para>
/// </remarks>
protected virtual void OnLockForkDetected()
{
}

/// <summary>
/// Invoked when the lock detects an internal error or illegal usage pattern that
/// indicates a serious flaw that should be immediately reported to the application
Expand Down Expand Up @@ -1072,6 +1090,7 @@ private bool TryIssueLock(Awaiter awaiter, bool previouslyQueued, bool skipPendi
{
bool issued = false;
bool isOrdinaryNestedLock = false; // ordinary nested lock is a nested lock always granted immediately. We don't need write ETW event to reduce noise in traces.
bool lockForkDetected = false;

lock (this.syncObject)
{
Expand All @@ -1097,6 +1116,21 @@ private bool TryIssueLock(Awaiter awaiter, bool previouslyQueued, bool skipPendi
switch (awaiter.Kind)
{
case LockKind.Read:
// Detect fork from upgradeable read context: the caller holds an active
// upgradeable read lock in its nesting chain but is executing on a thread
// that lacks the NonConcurrentSynchronizationContext, which typically means
// code used ConfigureAwait(false) and forked off the exclusive context.
// We skip this when hasWrite because the write fork path below already
// handles that case (with a hard failure), and we only report on the
// first request (!previouslyQueued) to avoid duplicate telemetry from
// PendAwaiter retries.
if (!previouslyQueued && hasUpgradeableRead && !hasWrite &&
this.CanCurrentThreadHoldActiveLock &&
!(SynchronizationContext.Current is NonConcurrentSynchronizationContext))
{
lockForkDetected = true;
}

if (this.issuedWriteLocks.Count == 0 && (skipPendingWriteLockCheck || this.waitingWriters.Count == 0))
{
issued = true;
Expand Down Expand Up @@ -1178,6 +1212,11 @@ private bool TryIssueLock(Awaiter awaiter, bool previouslyQueued, bool skipPendi
}
}

if (lockForkDetected)
{
this.OnLockForkDetected();
}

if (issued)
{
if (!isOrdinaryNestedLock)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4084,6 +4084,70 @@ await Assert.ThrowsAsync<InvalidOperationException>(async delegate
}
}

[Fact]
public async Task UpgradeableReadLockForksAndAsksForReadLock()
{
var lck = new LockWithForkDetection();
using (await lck.UpgradeableReadLockAsync())
{
await Task.Run(async delegate
{
// This requests a read lock from a thread pool thread (no NonConcurrentSynchronizationContext)
// while holding an upgradeable read lock. This is a fork of the exclusive context and should
// be detected via the OnLockForkDetected virtual method.
using (await lck.ReadLockAsync())
{
Assert.True(lck.IsReadLockHeld);
}
});
}

Assert.Equal(1, lck.ForkDetectedCount);
lck.Complete();
Assert.True(lck.Completion.Wait(UnexpectedTimeout));
}

[Fact]
public async Task UpgradeableReadLockDoesNotForkOnCorrectContext()
{
var lck = new LockWithForkDetection();
using (await lck.UpgradeableReadLockAsync())
{
// Request a nested read lock on the correct context (with NonConcurrentSynchronizationContext).
// This should NOT trigger fork detection.
using (await lck.ReadLockAsync())
{
Assert.True(lck.IsReadLockHeld);
}
}

Assert.Equal(0, lck.ForkDetectedCount);
lck.Complete();
Assert.True(lck.Completion.Wait(UnexpectedTimeout));
}

[Fact]
public async Task ReadLockForkFromReadOnlyContextDoesNotTriggerDetection()
{
var lck = new LockWithForkDetection();
using (await lck.ReadLockAsync())
{
await Task.Run(async delegate
{
// Fork from a read-only context. This should NOT trigger fork detection
// because read locks don't require NonConcurrentSynchronizationContext.
using (await lck.ReadLockAsync())
{
Assert.True(lck.IsReadLockHeld);
}
});
}

Assert.Equal(0, lck.ForkDetectedCount);
lck.Complete();
Assert.True(lck.Completion.Wait(UnexpectedTimeout));
}

[Fact]
public async Task WriteNestsReadWithWriteReleasedFirst()
{
Expand Down Expand Up @@ -5327,4 +5391,16 @@ public CriticalErrorException(Exception innerException)
{
}
}

private class LockWithForkDetection : AsyncReaderWriterLock
{
private int forkDetectedCount;

internal int ForkDetectedCount => this.forkDetectedCount;

protected override void OnLockForkDetected()
{
Interlocked.Increment(ref this.forkDetectedCount);
}
}
}
Loading