arcade icon indicating copy to clipboard operation
arcade copied to clipboard

Generate dumps for hanging tests

Open danmoseley opened this issue 5 years ago • 25 comments
trafficstars

Right now when a test hangs we do not get a dump file. Much of the time we need a dump (can't repro locally and code inspection doesn't show an obvious problem)

This mechanism needs to be a generic Helix mechanism - not tied to a particular repo's test infrastructure.

[Exception: In a few cases, in the libraries tests, we spawn a child process ("RemoteExec") and in those cases we can improve that infrastructure ourselves to get dumps on hangs. We have done this already on Windows and Linux is tracked here. ]

But in most cases, it's Helix infrastructure that needs to collect the dump.

Mono needs this also apparently: https://github.com/dotnet/runtime/issues/32325

danmoseley avatar Feb 27 '20 00:02 danmoseley

cc @stephentoub - I'm not really adding any value by opening this issue just FYI we now have one.

danmoseley avatar Feb 27 '20 00:02 danmoseley

Perhaps requires https://github.com/dotnet/core-eng/issues/9533#issuecomment-610595478

danmoseley avatar Apr 27 '20 17:04 danmoseley

cc @davidfowl

ViktorHofer avatar Apr 27 '20 17:04 ViktorHofer

Closing because the parent Epic was closed. If you believe this issue should still be worked on, please re-open.

michellemcdaniel avatar Feb 17 '22 22:02 michellemcdaniel

Looks like this still hasn't been completed.

agocke avatar Jun 22 '22 21:06 agocke

Current ask: it's very cheap for us to call our timeout kill() with SIGQUIT on non-Windows systems; marking for triage discussion

MattGal avatar Jun 22 '22 21:06 MattGal

AFAIK we did this. What doesn't work exactly?

davidfowl avatar Jun 22 '22 22:06 davidfowl

AFAIK we did this. What doesn't work exactly?

E.g. this job: https://dev.azure.com/dnceng/public/_build/results?buildId=1838524&view=logs&j=b2c675c1-1c32-50e9-7c40-e762459300db&t=4f51db95-f7de-5a4e-cd10-f078782dd75b

ended with

...
[EXECUTION TIMED OUT]
Exit Code:-3Executor timed out after 2700 seconds and was killed
['System.Runtime.Tests' END OF WORK ITEM LOG: Command timed out, and was killed]

But there's no crashdump for it to see where the process got stuck.

MichalStrehovsky avatar Jun 23 '22 01:06 MichalStrehovsky

Oh wait, the runtime uses a jank version of xunit right? It doesn't use the dotnet test runner.

davidfowl avatar Jun 23 '22 02:06 davidfowl

Yes, there's a custom runner. But unless the timeouting logic of dotnet test is in native code, I don't think that would help in situations like this (we suspect this one is a hang in GC suspension, which means any timeouting logic written in managed code likely wouldn't get a chance to run). We basically need the process to be killed in a way that produced a crashdump.

MichalStrehovsky avatar Jun 23 '22 02:06 MichalStrehovsky

@MichalStrehovsky the test host is a separate process so it would work just fine since it's the one collect a dump from the user process hosting test cases. The runtime doesn't use this currently and I thought there was a plan to move it over but I don't know where we are there.

cc @ViktorHofer

davidfowl avatar Jun 23 '22 02:06 davidfowl

We won't be able to use the test host for Native AOT anyway, at least not in near future.

I'm trying to chase it in low tech way here: https://github.com/dotnet/runtime/pull/71177. It's not great.

MichalStrehovsky avatar Jun 23 '22 02:06 MichalStrehovsky

We discussed this today in triage, and will not be moving forward with the "SIGQUIT" idea. While Helix machines will continue to happily collect dumps and provide them along with logs and result XMLs, the ROI and challenges of doing it from the Helix client remain too high to be worth it.

Some thoughts:

  • The entry point of the vast majority of helix work items is bash or cmd.exe. Getting a dump of this process is, like it was in the past, fairly useless.
  • Walking the entire tree of child processses and sigquit'ing each would produce copious dumps on every timeout in all of helix, which is undesirable from both a cost.

As runtime has already shown a way forward, and dotnet test can provide information (not just dumps) when tests hang, it does not make sense to try to force this behavior on every helix work item universally.

MattGal avatar Jul 08 '22 00:07 MattGal

@davidfowl

The runtime doesn't use this currently and I thought there was a plan to move it over but I don't know where we are there.

Unfortunately as long as VSTest depends on functionality that we would like avoid to depend on, i.e. the networking stack, we won't be able to use it.

See https://github.com/microsoft/vstest/issues/3595#issuecomment-1124921764 in which other devs like @jkotas shared concerns with depending on VSTest in its current state and what we would need.

IMO in an ideal world we would have an in-proc test runner entry point, generated with a source generator based on the test framework's attributes (i.e. Fact/Theory for xunit) to make a test invocation fast and reflection free. In addition to that, there should be an option to just print to stdout to avoid the IO dependency when it's not strictly needed.

ViktorHofer avatar Jul 08 '22 19:07 ViktorHofer

I don’t know if I agree with that. How does that work with VS? Is this a command line only story? Is this for customers or just the runtime (which has a unique problem)? Is this with things like nativeAOT in mind? I’m missing the big picture here

davidfowl avatar Jul 09 '22 15:07 davidfowl

How does that work with VS? Is this a command line only story?

VS test does not work for the inner loop if you work on the core runtime, for the reasons mentioned above. Command line only.

Is this for customers or just the runtime (which has a unique problem)? Is this with things like nativeAOT in mind?

It is primarily for the runtime, but the problems are not necessarily unique to the runtime.

For example, vstest does not have first class support for single-file testing (ie verifying that you library works in a single file deployment). We have custom solutions for number of these problems in the runtime.

jkotas avatar Jul 09 '22 18:07 jkotas

VS test does not work for the inner loop if you work on the core runtime, for the reasons mentioned above. Command line only.

FWIW, VS Test Explorer which uses VSTest underneath does work well in the inner loop for libraries developers (working on source code in this repository under src/libraries).

ViktorHofer avatar Jul 11 '22 15:07 ViktorHofer

As runtime has already shown a way forward,

I assume you mean https://github.com/dotnet/runtime/pull/71177 .. @MichalStrehovsky did that work?

We still need a way to get dumps from hangs in runtime unit tests. Reactivating while we discuss that and maybe when we have clarity we can move this to runtime.

@hoyosjs @mikem8361 @elinor-fung let us say that we are limiting the problem to getting a dump in this situation -- "long running test"

   System.Threading.Tasks.Dataflow.Tests: [Long Running Test] 'System.Threading.Tasks.Dataflow.Tests.TransformManyBlockTests.TestProducerConsumerAsyncEnumerable', Elapsed: 00:42:19
   System.Threading.Tasks.Dataflow.Tests: [Long Running Test] 'System.Threading.Tasks.Dataflow.Tests.TransformManyBlockTests.TestProducerConsumerAsyncEnumerable', Elapsed: 00:44:20
['System.Threading.Tasks.Dataflow.Tests' END OF WORK ITEM LOG: Command timed out, and was killed]

If we could make xunit send a SIGQUIT to the worker process, would that make a dump? Would we need the createdump variables set?

danmoseley avatar Jul 25 '22 20:07 danmoseley

We'd need the variables set. dotnet test used the diagnostics netcore client library to collect dumps on hang, but as they noted this is a VSTest feature. The library could be used in XUnit too or whatever we desired to request a point-in-time dump. Now sure mono implements that one. Also, that solution doesn't work for NativeAOT. Native AOT would require ulimit and rely on the OSs capability for dump collection.

hoyosjs avatar Jul 25 '22 20:07 hoyosjs

OK so work to make this happen for our unit tests would be

  • set env variables -- you are already working on that.
  • write code so that xunit could invoke the "diagnostics netcore client library" on hang
  • ensure "diagnostics netcore client library" is deployed ?

and vstest does all this, so we know it works?

If so - I can open an issue in dotnet/runtime.

This wouldn't help coreclr tests, right? If I understand right, those are standalone exe's so there is no harness to figure out something hung. (cc @trylek )

danmoseley avatar Jul 25 '22 21:07 danmoseley

This wouldn't help coreclr tests, right?

Also, it wouldn't help libraries tests in case the hang is in the runtime (GC, etc.).

jkotas avatar Jul 25 '22 21:07 jkotas

  • If the runtime is assumed to allow for event pipe requests and to be able to handle signals, the client library would work, or if there's a signal the environment variables could work.
  • If the runtime is assumed to be adversarial, calling createdump directly on the process ensures that all is captured. This requires SYS_CAP_PTRACE available on containers and elevation of an external watchdog (assuming the test runs in-proc). This is a coreclr-only solution. Often falling back on the OS for these cases is a good idea, although their dumps are not ideal.

hoyosjs avatar Jul 25 '22 22:07 hoyosjs

In runtime the one that launches the scripts acts as a watchdog of hangs and collects dumps on timeout.

hoyosjs avatar Jul 25 '22 22:07 hoyosjs

@hoyosjs is it reasonable to move this to an issue in dotnet/runtime? unless and until it becomes clear that there's some way to handle it centrally

danmoseley avatar Jul 26 '22 20:07 danmoseley

CoreCLR already implements this for the runtime tests in here: https://github.com/dotnet/runtime/blob/543bcc5ee7d6a2b9471b016770227421c43a756e/src/tests/Common/Coreclr.TestWrapper/CoreclrTestWrapperLib.cs#L207-L254. The vstest solution is better for upstack repos. Libraries would either need to use vstest (which I am not sure I recommend given they use a live-built coreclr), or have an external monitor grab dumps. In-proc xunit running the patched runtime as we currently do is not resilient against runtime issues. CoreCLR will likely need this too with the wrapper based approach.

hoyosjs avatar Aug 05 '22 20:08 hoyosjs