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

Issue 762006 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: Bug
Hotlist-MemoryInfra



Sign in to add a comment

DCHECK in ProcessMemoryDump::GetBlackHoleMad

Project Member Reported by siggi@chromium.org, Sep 5 2017

Issue description

This is turfing my SyzyASAN canary on a schedule, looks like all the processes take a DCHECK at the same time. See e.g. crash/7d193171cc34f8b2.

MemoryAllocatorDump* ProcessMemoryDump::GetBlackHoleMad() {
  DCHECK(is_black_hole_non_fatal_for_testing_);
  if (!black_hole_mad_)
    black_hole_mad_.reset(new MemoryAllocatorDump("discarded", this));
  return black_hole_mad_.get();
}

 

Comment 1 by siggi@chromium.org, Sep 5 2017

Cc: hjd@chromium.org
Owner: ssid@chromium.org
Hector, you've most recently been in that file. Any reason you can see why this is hitting today, and not on Friday?

Comment 2 by hjd@chromium.org, Sep 5 2017

Cc: kraynov@chromium.org
I'm suspicious of either: https://chromium.googlesource.com/chromium/src/+/0fdf71e1a7f5a5f57c5525df2252718702424bf4
or 
https://chromium-review.googlesource.com/c/chromium/src/+/643389

In terms of setup are you trying to use heap profiling of any kind or using memory-infra tracing? Or is is just crashing periodically while you're doing normal browsing?

+Greg:

Maybe we're calling https://cs.chromium.org/chromium/src/base/trace_event/process_memory_dump.cc?q=process_memory_dump.cc:305&sq=package:chromium&l=283 in background mode when we didn't used to?

Comment 3 by siggi@chromium.org, Sep 5 2017

Interesting - I run my SyzyASAN Canary with "--enable-heap-profiling=task-profiler", but note also that we're trying to ship a build with DCHECKs always on to that channel. See https://crbug.com/596231.
So the DCHECK and the crash are WAI. I wanted a crash precisely to get notified if things like this happen.
The real problem here is that --enable-heap-profiling=task-profiler is turning the full heap profiler (not just the shim) on, and this shouldn't happen.
Out of curiosity, is --enable-heap-profiling=task-profiler something that you turned on manually? Or is it shipped to the field via finch? I hope the former

Comment 5 by siggi@chromium.org, Sep 5 2017

I'm passing the flag explicitly to my Chrome.exe.
Cc: ssid@chromium.org
Labels: OS-Linux
Owner: lalitmag...@gmail.com
lalit@ just joined the team. How lucky, just in time for this bug.

> I'm passing the flag explicitly to my Chrome.exe.
Ok I feel better. There is something subtle going on. I followed the code, and we shouldn't be initializing the heap profiler machinery (that ultimately lead to that DCHECK) if we are in task-profiler mode. So there might be something like a typo or an uninitialized variable somewhere.
We just established this repros also on Linux.
Owner: hjd@chromium.org
temporarily moving this over to hjd@ until lalit gets a chromium.org account

Comment 8 by ssid@chromium.org, Sep 5 2017

Owner: ssid@chromium.org
Sorry, I am stealing this one. The issue is easy to spot. On registration we call OnHeapProfilingEnabled() when enabled with task profiler.
https://chromium-review.googlesource.com/c/chromium/src/+/643389

Maybe Lalith can try to fix  issue 735269  as a starter bug? I can help him if needed.
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 6 2017

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

commit 6158e4b01bdbe91b040a1d22b4a2f1a3541c9e83
Author: Siddhartha <ssid@chromium.org>
Date: Wed Sep 06 01:09:06 2017

Do not call OnHeapprofilingEnabled when task profiler is enabled

Only call OnHeapProfilingEnabled() when enabled with specific modes on
registering a new dump provider.

BUG= 762006 

Change-Id: Ifb461c1260fb5854b66d795a17403af3df02cc15
Reviewed-on: https://chromium-review.googlesource.com/651267
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499828}
[modify] https://crrev.com/6158e4b01bdbe91b040a1d22b4a2f1a3541c9e83/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/6158e4b01bdbe91b040a1d22b4a2f1a3541c9e83/base/trace_event/memory_dump_manager_unittest.cc

Status: Fixed (was: Available)
Thanks Lalit and Ssid for investigating and fixing this one !
Reopen if this shows up again
Test comment for monorail email notifications for this component, please ignore.

Sign in to add a comment