Annotate the intentional test-only leak in base/tracked_objects.cc.

Annotate the intentional test-only leak in base/tracked_objects.cc.
Remove several HeapChecker suppressions in favor of one annotation.

BUG=136936
R=jar@chromium.org
CC=glider@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=205190

[jar (doing other things), 2013-06-08 00:24:15]

I'll be surprised if this works.  Can you verify?

See comments below.

https://codereview.chromium.org/16387012/diff/1/base/tracked_objects.cc
File base/tracked_objects.cc (right):

https://codereview.chromium.org/16387012/diff/1/base/tracked_objects.cc#newco...
base/tracked_objects.cc:782: void ThreadData::ShutdownSingleThreadedCleanup(bool
leak) {
This function is *only* called during cleanup, which is ONLY called during
tests.

In most uses (real runs of chrome), this is never called, and is actually not
compiled into the final binary.

We do, in those cases, leak all data that was allocated, as we can't tell what
thread is alive and still using the data structures :-(.

The good news is that the total amount of data is well bounded (although with
efforts to support worker threads, we may have raised the bar above the initial
calculations... but the count of worker threads is still bounded.. so it all
stays pretty finite).

[earthdok_at_google, 2013-06-08 10:22:13]

> We do, in those cases, leak all data that was allocated, as we can't tell what
> thread is alive and still using the data structures :-(.

It's leaked in the sense that we never deallocate it, but it's still reachable
from all_thread_data_list_head_, right? HeapChecker will not report a leak in
that case. It's only in this function that we do actually make the data
unreachable.

> Can you verify?

Yes. Actually, HeapChecker doesn't report this leak at all, presumably due to
https://code.google.com/p/gperftools/issues/detail?id=537 However, I'm currently
deploying a new tool (LeakSanitizer), which does report this leak, and the
annotation suppresses it.

[jar (doing other things), 2013-06-10 03:34:54]

I see.... so this new tool detects "unreachable" entities as having leaked,
rather than the old "never freed" test.

LGTM

[earthdok_at_google, 2013-06-10 09:09:10]

Not exactly, they're both supposed to work the same way (i.e. reachability), the
old tool just happens to have a bug in this case.

[commit-bot: I haz the power, 2013-06-10 09:26:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/earthdok@google.com/16387012/15001

[commit-bot: I haz the power, 2013-06-10 11:32:23]

Change committed as 205190