New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 807631 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Xoogler
Closed: Feb 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

blink_heap_unittests failing on the ToTLinuxUBSanVptr bot

Project Member Reported by h...@chromium.org, Jan 31 2018

Issue description

Example build:
https://ci.chromium.org/buildbot/chromium.clang/ToTLinuxUBSanVptr/1483

Symbolized output:

../../third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:736:7: runtime error: member call on address 0x3e6ea9d4a098 which does not point to an object of typ
e 'blink::scheduler::TraceableState<bool, &blink::scheduler::kTracingCategoryNameInfo>'
0x3e6ea9d4a098: note: object is of type 'blink::scheduler::TraceableVariable'
 00 00 00 00  98 89 6c 0c 00 00 00 00  38 98 d4 a9 6e 3e 00 00  00 00 00 00 00 00 00 00  6d fc 31 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'blink::scheduler::TraceableVariable'
    #0 0x308d731 in blink::scheduler::RendererSchedulerImpl::OnShutdownTaskQueue(scoped_refptr<blink::scheduler::MainThreadTaskQueue> const&) third_party/WebKit/Source/platform/scheduler/
renderer/renderer_scheduler_impl.cc:736:7
    #1 0x3075ed4 in blink::scheduler::MainThreadTaskQueue::DetachFromRendererScheduler() third_party/WebKit/Source/platform/scheduler/renderer/main_thread_task_queue.cc:136:24
    #2 0x30761cc in blink::scheduler::MainThreadTaskQueue::ShutdownTaskQueue() third_party/WebKit/Source/platform/scheduler/renderer/main_thread_task_queue.cc:142:3
    #3 0x3074bda in blink::scheduler::MainThreadSchedulerHelper::~MainThreadSchedulerHelper() third_party/WebKit/Source/platform/scheduler/renderer/main_thread_scheduler_helper.cc:31:24
    #4 0x3083b59 in blink::scheduler::RendererSchedulerImpl::~RendererSchedulerImpl() third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:330:1
    #5 0x30840fc in blink::scheduler::RendererSchedulerImpl::~RendererSchedulerImpl() third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:298:49
    #6 0x44f359c in operator() buildtools/third_party/libc++/trunk/include/memory:2286:5
    #7 0x44f359c in reset buildtools/third_party/libc++/trunk/include/memory:2599
    #8 0x44f359c in ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2553
    #9 0x44f359c in content::TestBlinkWebUnitTestSupport::~TestBlinkWebUnitTestSupport() content/test/test_blink_web_unit_test_support.cc:210
    #10 0x44eb3ea in ~TestEnvironment content/public/test/blink_test_environment.cc:50:23
    #11 0x44eb3ea in content::TearDownBlinkTestEnvironment() content/public/test/blink_test_environment.cc:93
    #12 0x1502392 in ~BlinkTestEnvironmentScope third_party/WebKit/Source/platform/heap/RunAllTests.cpp:41:34
    #13 0x1502392 in runHelper(base::TestSuite*) third_party/WebKit/Source/platform/heap/RunAllTests.cpp:52
    #14 0x3f40be3 in Run base/callback.h:94:12
    #15 0x3f40be3 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::RepeatingCallback<int ()> const&, unsigned long, int, bool, base::RepeatingCallback<void ()> const&) base/t
est/launcher/unit_test_launcher.cc:220
    #16 0x3f40a29 in base::LaunchUnitTests(int, char**, base::RepeatingCallback<int ()> const&) base/test/launcher/unit_test_launcher.cc:558:10
    #17 0x1502466 in main third_party/WebKit/Source/platform/heap/RunAllTests.cpp:56:10
    #18 0x7fa29ae462b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
    #19 0x1319029 in _start (/work/chromium/src/out/debug/blink_heap_unittests+0x1319029)


I think what happens is that MainThreadOnly's dtor has already run when we receive the OnShutdownTaskQueue callback, which in turn is coming because RendererSchedulerImpl's dtor is running.

Maybe this can be fixed be reordering the members of RendererSchedulerImpl so the MainThreadOnly dtor runs later.
 

Comment 1 by h...@chromium.org, Feb 1 2018

Cc: h...@chromium.org
Owner: kraynov@chromium.org
Status: Assigned (was: Started)
It regressed here:

---
commit 167b9df7ff881a05a3ba648f9c55dccc6dbf735a
Author: Greg Kraynov <kraynov@chromium.org>
Date:   Thu Jan 11 23:42:59 2018 +0000

    Blink Scheduler: Registration for TraceableState and TraceableCounter.

    Automatic passing OnTraceLogEnabled event saves lines of code and
    helps not to worry about listing all the tracers.

    Bug:  798747 
    Change-Id: I371307e3d0d1b752e7975f75d6dd37e742af3833
    Reviewed-on: https://chromium-review.googlesource.com/852297
    Commit-Queue: Greg Kraynov <kraynov@chromium.org>
    Reviewed-by: Alexander Timin <altimin@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#528816}
---


Though one could argue that it was broken before, just that we didn't notice because before the TraceableVariable / State split.

Maybe another way to do this would to move was_shutdown out of MainThreadOnly and make it a regular variable, so that accessing it while the object is being destructed isn't undefined behavior. I assume the purpose of the variable is exactly to catch this "biting its own tail" problem.

Comment 2 by h...@chromium.org, Feb 5 2018

Labels: -Pri-3 Pri-1
Ping?
Cc: altimin@chromium.org
Looking
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b578720e875605be25aa21d69bfefd8eb316b208

commit b578720e875605be25aa21d69bfefd8eb316b208
Author: Greg Kraynov <kraynov@chromium.org>
Date: Tue Feb 06 00:22:59 2018

Blink Scheduler: Make was_shutdown safe to use in destructor.

Accessing destructed field is undefined behaviour.

Bug:  807631 
Change-Id: I88ac60b1b5ffe1a96e3f5a885527cba07d31652b
Reviewed-on: https://chromium-review.googlesource.com/901882
Commit-Queue: Greg Kraynov <kraynov@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534547}
[modify] https://crrev.com/b578720e875605be25aa21d69bfefd8eb316b208/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/b578720e875605be25aa21d69bfefd8eb316b208/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h

Comment 5 by h...@chromium.org, Feb 6 2018

Status: Fixed (was: Assigned)
The bot is green. Thank you!

Sign in to add a comment