GCTaskObserver collects heap pointers on stack in blink::testing::runPendingTasks |
||||
Issue description
As unittests are run outside of the message loop, tasks run in testing::runPendingTasks are executed as non-nested tasks. It's problematic because we often have heap pointers on stack.
struct A : public GarbageCollected<A> {
...
};
TEST(Foo, Bar)
{
A* a = new A;
postTask(some task);
testing::runPendingTasks();
// |a| is collected because ThreadHeap::collectGarbage is called with
// NoHeapPointerOnStack in runPendingTasks.
}
,
May 19 2016
Would it make sense to suppress a GC during testing::runPendingTasks()? This is doable by adding GCForbiddenScope to testing::runPendingTasks(). Another option would be to change the GC type to a conservative GC (HeapPointersOnStack), but I don't think it's a good idea because the conservative GC can bring undeterminism to the test result.
,
May 19 2016
> Would it make sense to suppress a GC during testing::runPendingTasks()? This is > doable by adding GCForbiddenScope to testing::runPendingTasks(). It works for me. I'm a bit concerned that we cannot call collectAllGarbage() in tasks for tests, but it looks no one is doing that.
,
May 19 2016
Let's go that way. We can also add ASSERT(!isGCForbidden()) to Heap::collectAllGarbage.
,
May 19 2016
> We can also add ASSERT(!isGCForbidden()) to Heap::collectAllGarbage. It's already added.
,
May 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0b9fcb3822616d2e067aa29192f1d4e4238db2f9 commit 0b9fcb3822616d2e067aa29192f1d4e4238db2f9 Author: yhirano <yhirano@chromium.org> Date: Thu May 19 12:00:30 2016 Forbid Oilpan GC in blink::testing::runPendingTasks As webkit unittests are run outside of the message loop, tasks run in blink::testing::runPendingTasks are executed as non-nested tasks. That means the registered GCTaskObserver will run GC with NoHeapPointerOnStack. This is problematic because we often have heap pointers on stack. This change forbid Oilpan GC in runPendingTasks to avoid such a problem. BUG= 613115 Review-Url: https://codereview.chromium.org/1995733003 Cr-Commit-Position: refs/heads/master@{#394743} [modify] https://crrev.com/0b9fcb3822616d2e067aa29192f1d4e4238db2f9/third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp
,
May 19 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by yhirano@chromium.org
, May 19 2016