Skip to content

Commit a0d882d

Browse files
committed
Fix headless cleanup race: dispose before signalling TCS (#20664)
Restructure DispatchCore<TResult> so application.Dispose() runs in a finally block before tcs.TrySet* is called. Previously, the using-var scope closed after TrySetResult, leaving s_uiThread set when the next non-headless test started. Adds a regression test in IsolationTests that verifies Dispatcher.UIThread is accessible from the calling thread immediately after a headless dispatch completes (PerTest isolation only).
1 parent 5fac590 commit a0d882d

2 files changed

Lines changed: 67 additions & 13 deletions

File tree

src/Headless/Avalonia.Headless/HeadlessUnitTestSession.cs

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,16 @@ internal Task<TResult> DispatchCore<TResult>(Func<Task<TResult>> action, bool ca
9696
using var globalCts = token.Register(s => ((CancellationTokenSource)s!).Cancel(), cts, true);
9797
using var localCts = cancellationToken.Register(s => ((CancellationTokenSource)s!).Cancel(), cts, true);
9898

99+
var application = _isolated
100+
? EnsureIsolatedApplication()
101+
: EnsureSharedApplication();
102+
103+
bool shouldCancel = false;
104+
Exception? caught = null;
105+
TResult result = default!;
106+
99107
try
100108
{
101-
using var application = _isolated
102-
? EnsureIsolatedApplication()
103-
: EnsureSharedApplication();
104109
var task = action();
105110
if (task.Status != TaskStatus.RanToCompletion)
106111
{
@@ -110,22 +115,36 @@ internal Task<TResult> DispatchCore<TResult>(Func<Task<TResult>> action, bool ca
110115

111116
if (cts.IsCancellationRequested)
112117
{
113-
tcs.TrySetCanceled(cts.Token);
114-
return;
118+
shouldCancel = true;
119+
}
120+
else
121+
{
122+
var frame = new DispatcherFrame();
123+
using var innerCts = cts.Token.Register(() => frame.Continue = false, true);
124+
Dispatcher.UIThread.PushFrame(frame);
125+
result = task.GetAwaiter().GetResult();
115126
}
116-
117-
var frame = new DispatcherFrame();
118-
using var innerCts = cts.Token.Register(() => frame.Continue = false, true);
119-
Dispatcher.UIThread.PushFrame(frame);
120127
}
121-
122-
var result = task.GetAwaiter().GetResult();
123-
tcs.TrySetResult(result);
128+
else
129+
{
130+
result = task.GetAwaiter().GetResult();
131+
}
124132
}
125133
catch (Exception ex)
126134
{
127-
tcs.TrySetException(ex);
135+
caught = ex;
128136
}
137+
finally
138+
{
139+
application.Dispose();
140+
}
141+
142+
if (caught != null)
143+
tcs.TrySetException(caught);
144+
else if (shouldCancel)
145+
tcs.TrySetCanceled(cts.Token);
146+
else
147+
tcs.TrySetResult(result);
129148
}, executionContext));
130149
return tcs.Task;
131150
}

tests/Avalonia.Headless.UnitTests/IsolationTests.cs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
using System;
22
using System.Reflection;
3+
using System.Threading;
4+
using System.Threading.Tasks;
5+
using Avalonia.Headless;
36
using Avalonia.Threading;
47

58
namespace Avalonia.Headless.UnitTests;
@@ -64,4 +67,36 @@ public void Application_Instance_Should_Match_Isolation_Level(int runIndex)
6467
s_previousAppRef = new WeakReference<Application>(currentApp);
6568
s_previousDispatcherRef = new WeakReference<Dispatcher>(currentDispatcher);
6669
}
70+
71+
#if NUNIT
72+
[Test]
73+
#elif XUNIT
74+
[Fact]
75+
#endif
76+
public async Task Dispatch_Cleanup_Should_Complete_Before_Task_Returns()
77+
{
78+
// Regression test for https://github.com/AvaloniaUI/Avalonia/issues/20664.
79+
// EnsureIsolatedApplication().Dispose() must complete (resetting s_uiThread to null)
80+
// before the dispatch task resolves. If Dispose ran after tcs.TrySetResult, s_uiThread
81+
// could still point to the headless dispatcher here, making CheckAccess() return false
82+
// on this non-headless thread.
83+
//
84+
// Only applies to PerTest isolation: PerAssembly uses EnsureSharedApplication which
85+
// intentionally keeps s_uiThread set for the lifetime of the assembly, so CheckAccess()
86+
// from a non-headless thread would always be false there and the race does not apply.
87+
var isolationLevel =
88+
GetType().Assembly.GetCustomAttribute<AvaloniaTestIsolationAttribute>()?.IsolationLevel
89+
?? AvaloniaTestIsolationLevel.PerTest;
90+
91+
if (isolationLevel != AvaloniaTestIsolationLevel.PerTest)
92+
return;
93+
94+
// Uses the shared assembly session (not StartNew) so no competing thread calls
95+
// ResetGlobalState() concurrently with other tests in the suite.
96+
var session = HeadlessUnitTestSession.GetOrStartForAssembly(GetType().Assembly);
97+
98+
await session.Dispatch(() => { }, CancellationToken.None);
99+
100+
AssertHelper.True(Dispatcher.UIThread.CheckAccess());
101+
}
67102
}

0 commit comments

Comments
 (0)