Assert failure in sgen-mono.c while running System.Drawing tests

[stephentoub]

https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-44136-merge-c86262a09b5b4ea99f/System.Drawing.Common.Tests/console.5aca01fc.log?sv=2019-07-07&se=2020-11-22T19%3A39%3A42Z&sr=c&sp=rl&sig=D2w6NnhCPSVf5cfFCGA%2BU%2By6nJ5llAgjBphBA5DUxMk%3D

    System.Drawing.Drawing2D.Tests.ColorBlendTests.Ctor_LargeCount_ThrowsOutOfMemoryException [SKIP]
      Condition(s) not met: "IsNotIntMaxValueArrayIndexSupported"
* Assertion at F:\workspace\_work\1\s\src\mono\mono\metadata\sgen-mono.c:2375, condition `stack == NULL || mono_handle_stack_is_empty (stack)' not met

[ghost]

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

[ghost]

Tagging subscribers to this area: @BrzVlad
See info in area-owners.md if you want to be subscribed.

[CoffeeFlux]

@BrzVlad can you take a look? If it doesn't seem like a GC issue feel free to bounce it back.

[BrzVlad]

1. This doesn't look like a GC issue, rather an icall not clearing its handle stack
2. I ran this suite 3000 times on windows without getting any crashes
3. This is the list of icalls called by the suite, with their call count https://gist.github.com/BrzVlad/d2f121bfffd3aa0cda6bf8443acf893b
4. I remember seeing this assertion in the past. Maybe @lambdageek has some more ideas

[ghost]

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

[lambdageek]

bah we should change

				g_assert (stack == NULL || mono_handle_stack_is_empty (stack));

to

				g_assertf (stack == NULL || mono_handle_stack_is_empty (stack), "skipped thread %p (reason %d) has non-empty handle stack", info, skip_reason);

so that we get a better assert message.

---

So there are two things happening here: we decided to skip the thread from a stop-the-world suspend for some reason, and the thread has a non-empty handle stack.

On Windows one reason we might skip a thread is if SuspendThread failed - which can happen transiently, I think - in that case we abort the attempt to suspend and skip the thread during STW (mono/mono#15486).

If that's what's happening, it's not surprising that the thread had a non-empty handle stack. it could have been doing any arbitrary work when we tried to preempt it.

---

I think this is another piece of evidence that we need to be able to back out of the whole STW and start over. (Similar to the pthread_kill transient failures - #32377 (comment)). Skipping the thread just invites the GC to miss roots. /cc @lateralusX

[SamMonoRT]

Moving to 7.0.0

[SamMonoRT]

Moving to 8.0.0 and assigning to @lateralusX

[lateralusX]

Closing this issue for a couple of different reasons:

• The issue didn't reproduce over a large number of runs.
• A number of fixes have been done to get the library tests running on Windows after this issue was filed, CoreCLR runtime tests on Mono Windows x64. #64281, one of them explicitly fixed a issue in System.Drawing related tests.
• The component and tests have moved out of .net8 repository.