PersistentHeapVector::operator= causes crashes in unittests |
|||
Issue descriptionIt appears that using PersistentHeapVector::operator= will cause a crash, at least in unittests (not tried in 'real' code). When trying to do this in a CL (https://chromium-review.googlesource.com/c/chromium/src/+/1060693/2/third_party/blink/renderer/core/animation/compositor_animations_test.cc#85 , lines 85 and 111), I got: Received signal 11 <unknown> 000000000000 #0 0x00000428fb3c base::debug::StackTrace::StackTrace() #1 0x00000428f6b1 base::debug::(anonymous namespace)::StackDumpSignalHandler() #2 0x7fc5a0715330 <unknown> #3 0x00000243ec2b blink::MarkingVisitor::Visit() #4 0x00000243ed18 blink::MarkingVisitor::VisitBackingStoreStrongly() #5 0x000002d769e0 blink::TraceMethodDelegate<>::Trampoline() #6 0x000002440845 blink::PersistentNode::TracePersistentNode() #7 0x00000244043c blink::PersistentRegion::TracePersistentNodes() #8 0x000002442d59 blink::ThreadState::VisitPersistents() #9 0x00000242dcb2 blink::ThreadHeap::VisitPersistentRoots() #10 0x000002449077 blink::ThreadState::MarkPhaseVisitRoots() #11 0x0000024427a4 blink::ThreadState::CollectGarbage() #12 0x000004470652 blink::V8GCController::GcEpilogue() #13 0x000001e756e4 v8::internal::Heap::PerformGarbageCollection() #14 0x000001e7326b v8::internal::Heap::CollectGarbage() #15 0x000001e719f4 v8::internal::Heap::CollectAllGarbage() #16 0x000004470cd0 blink::V8GCController::CollectAllGarbageForTesting() #17 0x00000134c35b (anonymous namespace)::runHelper() #18 0x000006563780 base::(anonymous namespace)::LaunchUnitTestsInternal() #19 0x0000065635d0 base::LaunchUnitTests() #20 0x00000134c1bd main #21 0x7fc59f28df45 __libc_start_main From a conversation with jbroman: "it looks to me like PersistentHeapCollectionBase doesn't do anything to make move-assignment work correctly, and so the persistent node state is getting messed up" I was able to work around this by just passing a reference instead.
,
May 21 2018
Keishi, since you have been looking into this recently, can you check the issue?
,
May 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e6c31a05fac200787d40043f00b046efb788ea0e commit e6c31a05fac200787d40043f00b046efb788ea0e Author: Keishi Hattori <keishi@chromium.org> Date: Wed May 23 05:35:13 2018 Oilpan: Fix copy assignment for PersistentHeapCollections PersistentHeapCollectionBase::persistent_node_ was being copied on copy assignment. PersistentHeapCollectionBase::persistent_node_ should never change once assigned. When they get overwritten, the old PersistentNode might point to a freed PersistentHeapCollection. Bug: 843550 Change-Id: Ic2397683c346ba019a24bfe9c241f6bcd81a1c2d Reviewed-on: https://chromium-review.googlesource.com/1068557 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Keishi Hattori <keishi@chromium.org> Cr-Commit-Position: refs/heads/master@{#560956} [modify] https://crrev.com/e6c31a05fac200787d40043f00b046efb788ea0e/third_party/blink/renderer/platform/heap/heap_test.cc [modify] https://crrev.com/e6c31a05fac200787d40043f00b046efb788ea0e/third_party/blink/renderer/platform/heap/persistent.h
,
May 23 2018
I confirmed locally that the unit tests from https://chromium-review.googlesource.com/c/chromium/src/+/1060693/2/ run after this fix. Please try it out and let me know if you have any issues. |
|||
►
Sign in to add a comment |
|||
Comment 1 by jbroman@chromium.org
, May 16 2018