Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

xUnit is crashing due a NET's EventListener race #6358

Closed
davkean opened this issue Oct 27, 2015 · 7 comments
Closed

xUnit is crashing due a NET's EventListener race #6358

davkean opened this issue Oct 27, 2015 · 7 comments
Labels
Area-Interactive Blocked Bug

Comments

@davkean
Copy link
Member

davkean commented Oct 27, 2015

Tests are often failing with:

System.InvalidOperationException: Collection was modified; enumeration operation
 may not execute.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resour
ce)
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at System.Collections.Generic.List`1.Enumerator.MoveNext()
   at System.Diagnostics.Tracing.EventListener.DisposeOnShutdown(Object sender,
EventArgs e)

This a .NET bug; dotnet/corefx#3793, however, we're not going to see a fix soon. I've got an ask out to xUnit to see if we can submit a PR to workaround the issue: xunit/xunit#646

If not, then we might need build our own version of xUnit and work around it manually. We're hitting this ~2 - 3 every 50 runs.
This was referenced Oct 28, 2015
Closed
Closed
@jaredpar jaredpar mentioned this issue Oct 29, 2015
Closed
@natidea natidea mentioned this issue Oct 29, 2015
Merged
jaredpar added a commit to jaredpar/roslyn that referenced this issue Oct 31, 2015
@jaredpar
Disable AppDomain isolation in unit tests
24b0756 
This change disables AppDomain isolation in the unit tests that don't require it.  This is necessary for our goal of running many of the suites on CoreCLR and helps reduce our exposure to dotnet#6358.

The AppDomain creation code incorrectly used AppDomain.CurrentDomain.BaseDirectory as the new base directory.  There is no gurantee this points to the directory containing the unit test binaries and this is actually the case in xunit without AppDomains.  Instead it points to the directory containing xunit.oconsole.x86 and uses AppDomain.ResolveAssembly / LoadFrom to resolve unit test binaries.

Switching over to no AppDomains in xunit was causing the creation of RuntimeAssemblyManager across AppDomains to fail.  The error given was a failure to load Roslyn.Test.Utilities.Desktop in the original AppDomain.  This is odd given that the code is executing inside Roslyn.Test.Utilities.Desktop.  The error was fixed by hooking AppDomain.AssemblyResolve in the original AppDomain and essentially reloading the DLL.  I'm not completely sure why this worked but I suspect it has to do with the original AppDomain having an incorrect base directory.
@jaredpar jaredpar mentioned this issue Oct 31, 2015
Merged
@jaredpar jaredpar changed the title [Flaky Test] xUnit is crashing due a NET's EventListener race xUnit is crashing due a NET's EventListener race Nov 2, 2015
@jaredpar
Copy link
Member

jaredpar commented Nov 2, 2015

To followup after some investigation into the xunit code base we determined there is no real work around they can do for this bug. xunit is just running into this issue through no fault of their own and there is little they can do to avoid hitting it.

@jaredpar jaredpar mentioned this issue Nov 2, 2015
Closed
@genlu genlu mentioned this issue Nov 12, 2015
Closed
@jaredpar
Copy link
Member

jaredpar commented Dec 7, 2015

It's been about 1 month since we removed AppDomain usage from the majority of our test suites: in particular the suites that were subject to this issue. Haven't seen any occurrences of this recently, resolving.

@jaredpar jaredpar closed this as completed Dec 7, 2015
@balajikris balajikris mentioned this issue Dec 14, 2015
Merged
@davkean davkean reopened this Dec 18, 2015
@davkean davkean mentioned this issue Dec 18, 2015
Merged
@davkean
Copy link
Member Author

davkean commented Dec 18, 2015

We're still running into this with the scripting tests. I've submitted a PR as a workaround, let's use this bug to track removing the workaround.

@jaredpar
Copy link
Member

@davkean i looked at the scripting tests and it looks like they are still using AppDomain instances for test isolation. From app.config:

    <add key="xunit.appDomain" value="required"/>

This is why scripting is still seeing this issue. If this is switched to denied then the bug won't be a factor anymore because xunit won't be dealing with AppDomain tear down races.

Do we have a bug changing scripting to no longer using AppDomain here? That seems like the best fix we can do. It's already been done in many other layers and is good practice for our move to CoreCLR.

@dpoeschl dpoeschl mentioned this issue Dec 23, 2015
Closed
@tmat tmat mentioned this issue Dec 24, 2015
Closed
This was referenced Jan 14, 2016
Closed
Merged
davkean added a commit to davkean/roslyn that referenced this issue Jan 14, 2016
@davkean
Disable AppDomain isolation in unit tests
aa4a98c 
This change disables AppDomain isolation in the unit tests that don't require it.  This is necessary for our goal of running many of the suites on CoreCLR and helps reduce our exposure to dotnet#6358.
@davkean davkean mentioned this issue Jan 15, 2016
Closed
davkean added a commit to davkean/roslyn that referenced this issue Jan 22, 2016
@davkean
Turn off AppDomain isolation in xunit to fix dotnet#8060.
30ddc26 
This will cause us to avoid hitting the EventListener bug:
dotnet#6358.
This was referenced Jan 22, 2016
Merged
Merged
@davkean davkean mentioned this issue Feb 20, 2016
Closed
@davkean davkean mentioned this issue Mar 16, 2016
Merged
@joshfree
Copy link
Member

Hit this issue recently

Build #159 (Apr 13, 2016 2:26:01 AM)
http://dotnet-ci.cloudapp.net/job/dotnet_corefx/job/outerloop_windows_nt_debug/159/

@jaredpar
Copy link
Member

@joshfree if you're interested in a work around for this framework bug see this commit: 958ca00

@JonHanna JonHanna mentioned this issue Apr 17, 2016
Merged
@davidsh davidsh mentioned this issue Apr 26, 2016
Merged
@ManishJayaswal
Copy link
Contributor

Closing this as this issue is resolved.

davkean added a commit to dotnet/project-system that referenced this issue May 3, 2016
@davkean
Turn off AppDomain isolation in xunit to fix #8060.
6f5a1e3 
This will cause us to avoid hitting the EventListener bug:
dotnet/roslyn#6358.
@davkean davkean mentioned this issue Jan 5, 2017
Closed
@davidsh davidsh mentioned this issue May 20, 2017
Merged
@tijoytom-zz tijoytom-zz mentioned this issue May 24, 2017
Merged
@morganbr morganbr mentioned this issue May 25, 2017
Merged
@steveharter steveharter mentioned this issue May 25, 2017
Merged
@vrohit13 vrohit13 mentioned this issue Aug 9, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Interactive Blocked Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jaredpar @davkean @joshfree @ManishJayaswal