Skip to content

Commit 9777164

Browse files
Fix race in TestContextImplementation by @Youssef1313 in #6249 (backport to rel/3.10) (#6250)
Co-authored-by: Youssef1313 <youssefvictor00@gmail.com>
1 parent 02c41cf commit 9777164

2 files changed

Lines changed: 48 additions & 9 deletions

File tree

src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,27 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
2727
#endif
2828
public class TestContextImplementation : TestContext, ITestContext, IDisposable
2929
{
30+
internal sealed class SynchronizedStringBuilder
31+
{
32+
private readonly StringBuilder _builder = new();
33+
34+
[MethodImpl(MethodImplOptions.Synchronized)]
35+
internal void Append(char value)
36+
=> _builder.Append(value);
37+
38+
[MethodImpl(MethodImplOptions.Synchronized)]
39+
internal void Append(string? value)
40+
=> _builder.Append(value);
41+
42+
[MethodImpl(MethodImplOptions.Synchronized)]
43+
internal void Append(char[] buffer, int index, int count)
44+
=> _builder.Append(buffer, index, count);
45+
46+
[MethodImpl(MethodImplOptions.Synchronized)]
47+
public override string ToString()
48+
=> _builder.ToString();
49+
}
50+
3051
private static readonly AsyncLocal<TestContextImplementation?> CurrentTestContextAsyncLocal = new();
3152

3253
/// <summary>
@@ -45,9 +66,9 @@ public class TestContextImplementation : TestContext, ITestContext, IDisposable
4566
private readonly IMessageLogger? _messageLogger;
4667
private readonly CancellationTokenRegistration? _cancellationTokenRegistration;
4768

48-
private StringBuilder? _stdOutStringBuilder;
49-
private StringBuilder? _stdErrStringBuilder;
50-
private StringBuilder? _traceStringBuilder;
69+
private SynchronizedStringBuilder? _stdOutStringBuilder;
70+
private SynchronizedStringBuilder? _stdErrStringBuilder;
71+
private SynchronizedStringBuilder? _traceStringBuilder;
5172
private StringBuilder? _testContextMessageStringBuilder;
5273

5374
private bool _isDisposed;
@@ -394,21 +415,21 @@ internal void WriteTrace(char value)
394415
internal void WriteTrace(string? value)
395416
=> GetTraceStringBuilder().Append(value);
396417

397-
private StringBuilder GetOutStringBuilder()
418+
private SynchronizedStringBuilder GetOutStringBuilder()
398419
{
399-
_ = _stdOutStringBuilder ?? Interlocked.CompareExchange(ref _stdOutStringBuilder, new StringBuilder(), null)!;
420+
_ = _stdOutStringBuilder ?? Interlocked.CompareExchange(ref _stdOutStringBuilder, new SynchronizedStringBuilder(), null)!;
400421
return _stdOutStringBuilder;
401422
}
402423

403-
private StringBuilder GetErrStringBuilder()
424+
private SynchronizedStringBuilder GetErrStringBuilder()
404425
{
405-
_ = _stdErrStringBuilder ?? Interlocked.CompareExchange(ref _stdErrStringBuilder, new StringBuilder(), null)!;
426+
_ = _stdErrStringBuilder ?? Interlocked.CompareExchange(ref _stdErrStringBuilder, new SynchronizedStringBuilder(), null)!;
406427
return _stdErrStringBuilder;
407428
}
408429

409-
private StringBuilder GetTraceStringBuilder()
430+
private SynchronizedStringBuilder GetTraceStringBuilder()
410431
{
411-
_ = _traceStringBuilder ?? Interlocked.CompareExchange(ref _traceStringBuilder, new StringBuilder(), null)!;
432+
_ = _traceStringBuilder ?? Interlocked.CompareExchange(ref _traceStringBuilder, new SynchronizedStringBuilder(), null)!;
412433
return _traceStringBuilder;
413434
}
414435

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/TestContextImplementationTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,4 +407,22 @@ public void DisplayMessageShouldForwardToIMessageLogger()
407407
messageLoggerMock.Verify(x => x.SendMessage(TestMessageLevel.Warning, "WarningMessage"), Times.Once);
408408
messageLoggerMock.Verify(x => x.SendMessage(TestMessageLevel.Error, "ErrorMessage"), Times.Once);
409409
}
410+
411+
public void WritesFromBackgroundThreadShouldNotThrow()
412+
{
413+
var testContextImplementation = new TestContextImplementation(_testMethod.Object, _properties, new Mock<IMessageLogger>().Object, testRunCancellationToken: null);
414+
var t = new Thread(() =>
415+
{
416+
for (int i = 0; i < 100; i++)
417+
{
418+
testContextImplementation.WriteConsoleOut(new string('a', 1000000));
419+
testContextImplementation.WriteConsoleErr(new string('b', 1000000));
420+
}
421+
});
422+
423+
t.Start();
424+
_ = testContextImplementation.GetOut();
425+
_ = testContextImplementation.GetErr();
426+
t.Join();
427+
}
410428
}

0 commit comments

Comments
 (0)