AArnott
commented
4 years ago Thanks for filing. FWIW, if there's pushback on this, we can investigate making xunit.stafact more resilient to the SynchronizationContext being disposed.
Open weltkante opened 4 years ago
AArnott
commented
4 years ago Thanks for filing. FWIW, if there's pushback on this, we can investigate making xunit.stafact more resilient to the SynchronizationContext being disposed.
weltkante
commented
4 years ago I don't think theres much you can do on the stafact side. There is no guarantee that the test is done at the point where the SC is disposed and just stopping to pump messages can lead to deadlocks.
Also for any workaround in stafact there is the whole issue of determining if the SC (or rather, the underlying infrastructure) is disposed, the unit test has its own instance of WFSC but its sharing the same WinForms infrastructure with xunit.stafact so disposing either SC will make the other inoperable. (Thats why making WFSC disposable is bad API design, its not the owner of the underlying infrastructure.)
The right thing to do is not disposing the infrastructure you're running on during a test. As far as the test is concerned there is no strong reason for these to be WinFormsFact/Theory, but when I tried turning them into StaFact/Theory that just makes the tests explode differently (not in xunit.stafact but in the test itself). I don't know exactly why that wasn't a problem before, but it indicates that the way the tests are written is wrong.
RussKie
commented
4 years ago Not sure if it is related, but just observed a failure in https://dev.azure.com/dnceng/public/_build/results?buildId=677815&view=results
XUnit : error : Tests failed: F:\workspace\_work\1\s\artifacts\log\Release\System.Windows.Forms.Tests_netcoreapp5.0_x86.log [netcoreapp5.0|x86] [F:\workspace\_work\1\s\src\System.Windows.Forms\tests\UnitTests\System.Windows.Forms.Tests.csproj]Very little information unfortunately:
Unhandled exception. System.InvalidOperationException: Invoke or BeginInvoke cannot be called on a control until the window handle has been created.
at System.Windows.Forms.Control.MarshaledInvoke(Control caller, Delegate method, Object[] args, Boolean synchronous) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 6870
at System.Windows.Forms.Control.BeginInvoke(Delegate method, Object[] args) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 4685
at System.Windows.Forms.WindowsFormsSynchronizationContext.Post(SendOrPostCallback d, Object state) in /_/src/System.Windows.Forms/src/System/Windows/Forms/WindowsFormsSynchronizationContext.cs:line 91
at Xunit.Sdk.Utilities.SyncContextAwaiter.OnCompleted(Action continuation)
at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AwaitUnsafeOnCompleted[TAwaiter](TAwaiter& awaiter, IAsyncStateMachineBox box)
--- End of stack trace from previous location ---
at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__140_1(Object state)
at System.Threading.QueueUserWorkItemCallbackDefaultContext.Execute()
at System.Threading.ThreadPoolWorkQueue.Dispatch()
at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback()Yeah I'm getting that locally as well from time to time, stack trace looks like its the same root cause (disposed WFSC) but probably not doing it intentionally because then it would fail reliably. For what its worth I actually captured this one in a full dump while trying to repro the other random crashes.
I don't know if its possible to read off the dump where the exception is coming from, the exception seems to be rethrown in the thread pool, meaning the managed debugger loses any connection to the failed task. Maybe someone from the runtime team can take a look and tell if its possible to recover any information about the failing task/thread (task my refer to variables identifying the test; thread name contains the test name if this was thrown on a test thread)
RussKie
commented
4 years ago RussKie
commented
4 years ago Observed in https://dev.azure.com/dnceng/public/_build/results?buildId=726290&view=results for https://github.com/dotnet/winforms/pull/3563 🤔 Likely a red herring...
Tanya-Solyanik
commented
1 month ago @LeafShi1 - please see if you are already tracking this test failure, if you have another bug, please link it here and close this one.
LeafShi1
commented
1 month ago @LeafShi1 - please see if you are already tracking this test failure, if you have another bug, please link it here and close this one.
@Tanya-Solyanik This is a fixed error. We have not created other issue to track them. Even if #3122 has been merged, the three cases are still fail.
.NET Core Version: master branch
Have you experienced this same bug with .NET Framework?: no
Problem description:
Following three tests are blocking the update of xunit.stafact (#3122) and will be skipped by that PR unless they get fixed before the package update PR gets merged.
They are a WinFormsFact/Theory and dispose the SynchronizationContext early. This leads to either crashing the whole process in debug builds due to a failed assert, or failing tests in release builds. Reason is that WinFormsFact/Theory after the package update will pump messages via DoEvents, calling this after the SC gets disposed will lead to an exception (or crash in debug build).
(There may also be the question if this being an assert is the correct choice, maybe its worth checking explicitly for the current thread being a disposed thread, to give a useful exception instead. Or maybe its time to get rid of the public Dispose on WindowsFormsSynchronizationContext, it feels inappropriate since the WFSC is not the owner of the underlying infrastructure, there can be any number of WFSC and disposing one is terminating the infrastructure for all of them, thats not how the disposable pattern is supposed to work.)
The error gets blamed onto stafact infrastructure, but the tests are causing it by disposing early. Failed CI build can be found here: https://github.com/dotnet/winforms/runs/681811749
Debug build:
Release build:
Expected behavior: Tests using WinFormsFact/Theory should not dispose the infrastructure they are running on
Minimal repro: Execute one of those tests after updating xunit.stafact to
1.0.33-betaor newer/cc: @hughbe @RussKie