src: force gc for flaky (deadlock) tests#57476
src: force gc for flaky (deadlock) tests#57476jakecastelli wants to merge 1 commit intonodejs:mainfrom
Conversation
|
Review requested:
|
b510857 to
9f8370d
Compare
9f8370d to
f8fb19d
Compare
This patch adds a new flag `force_gc_for_test` (internal use for test purpose only) to force GC specifically for flaky tests that suffer from deadlock. The deadlock occurs when: 1. Main thread calls DrainTasks and executes worker tasks 2. Worker task needs memory allocation requiring GC 3. GC must run on main thread, but main thread is busy running the worker task
f8fb19d to
43781fb
Compare
If we need to update tests one by one we don't need a new flag. We can just call |
|
I also fear that if we did this for all tests we would end up ignoring the underlying issue because it would no longer be a problem. So far only a handful of people have investigated the issue and apparently it is not trivial (#56827). When we no longer have a constant reminder that it exists via CI, it will be forgotten. |
|
Fair points, I will continue to dig the underlining issue with the rest of us, and hopefully we can get it properly fixed. |
This patch adds a new flag
force_gc_for_test(internal use for test purpose only) to force GC for flaky tests that suffer from deadlock. Refs: #54918 and #56827 (comment)The deadlock occurs when (correct me if I am wrong):
Notes:
RequestGarbageCollectionForTestingwill require--expose-gcflag to work which means for the flaky tests that suffer from deadlock we will need// Flags: --expose-gc --force-gc-for-testat the top of the test file.