[PATCH] D129004: [Support] Fix Windows dump file hang with multi-threaded crashes

Wed Jul 6 16:12:22 PDT 2022

rnk added a comment.

The change seems reasonable, but I hesitate to add the test as is.

================
Comment at: llvm/unittests/Support/WindowsDumpFileTest.cpp:41
+
+  if (getenv("LLVM_WINDOWS_DUMP_FILE_TEST_MULTI_THREAD")) {
+    ASSERT_FALSE(sys::Process::AreCoreFilesPrevented());
----------------
I appreciate the effort and thoroughness that went into writing this test, but I don't think we should commit it as is.

If the critical section is removed, this test will nondeterministically fail with a timeout due to the deadlock. That's not something we can use to bisect problems or debug why things went wrong. I don't think it's worth spending resources to run this test if it can't produce actionable results from a buildbot.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129004/new/

https://reviews.llvm.org/D129004