net: Prevent fuzzers from leaking on each iteration.

net:  Prevent fuzzers from leaking on each iteration.

While the UMA macros use statics to avoid leaking, code that directly
invokes base::Histogram::FactoryGet will keep on creating new objects
on each iteration, unless the StatisticsRecorder has been initialized.
This CL modifies net's Fuzzer infrastructure to ensure it is
initialized.

BUG=602440
Committed: https://crrev.com/0960f7561368543f46cc126d64820307728faa2f
Cr-Commit-Position: refs/heads/master@{#391039}

[mmenke, 2016-05-02 18:38:06]

mmenke@chromium.org changed reviewers:
+ eroman@chromium.org

[mmenke, 2016-05-02 18:38:07]

Haven't tested this doesn't under ASAN, but have confirmed that this stops
Histogram::Factory::Build from creating new histogram objects.

[mmenke, 2016-05-02 18:38:29]

On 2016/05/02 18:38:07, mmenke wrote:
> Haven't tested this doesn't under ASAN, but have confirmed that this stops
> Histogram::Factory::Build from creating new histogram objects.

"doesn't leak every iteration under ASAN", rather

[eroman, 2016-05-02 19:09:15]

lgtm

[mmenke, 2016-05-02 19:13:55]

Description was changed from

==========
net:  Prevent fuzzers from leaking on each fuzzer iteration.

While the UMA macros use statics to avoid leaking, code that directly
calls that directly invoke base::Histogram::FactoryGet will keep on
creating new objects, unless the StatisticsRecorder has been
initialized. This CL modifies net's Fuzzer code to ensure it is
initialized.

BUG=602440
==========

to

==========
net:  Prevent fuzzers from leaking on each iteration.

While the UMA macros use statics to avoid leaking, code that directly
calls that directly invoke base::Histogram::FactoryGet will keep on
creating new objects, unless the StatisticsRecorder has been
initialized. This CL modifies net's Fuzzer code to ensure it is
initialized.

BUG=602440
==========

[eroman, 2016-05-02 19:14:07]

As an aside, this seems like a very mysterious way for histograms to work.

Haven't dug into the specifics, but somewhat troubling that it would behave in
this manner rather than crashing or no-op ing...

[mmenke, 2016-05-02 19:14:41]

Description was changed from

==========
net:  Prevent fuzzers from leaking on each iteration.

While the UMA macros use statics to avoid leaking, code that directly
calls that directly invoke base::Histogram::FactoryGet will keep on
creating new objects, unless the StatisticsRecorder has been
initialized. This CL modifies net's Fuzzer code to ensure it is
initialized.

BUG=602440
==========

to

==========
net:  Prevent fuzzers from leaking on each iteration.

While the UMA macros use statics to avoid leaking, code that directly
invokes base::Histogram::FactoryGet will keep on creating new objects
on each iteration, unless the StatisticsRecorder has been initialized.
This CL modifies net's Fuzzer infrastructure to ensure it is
initialized.

BUG=602440
==========

[mmenke, 2016-05-02 19:14:54]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2016-05-02 19:15:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1941933002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1941933002/1

[mmenke, 2016-05-02 19:16:47]

I agree...This behavior (And even ownership semantics) aren't at all documented
in the histogram APIs.

[+isherman, +asvitkine]:  FYI.

[Alexei Svitkine (slow), 2016-05-02 19:21:21]

asvitkine@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Alexei Svitkine (slow), 2016-05-02 19:21:21]

I'd welcome a change to add more explanation of this on comments on FactoryGet()
functions.

Though in this case, not sure those would be useful since I assume this test
suite is just exercising normal Chrome code - so the person adding a new test
suite would need to know this rather than the person using FactoryGet methods.

I wonder if it would make sense to have a DCHECK when they're invoked with
StatisticsRecorder being initialized? Though I wonder what kind of other
problems that would uncover.

[eroman, 2016-05-02 19:31:31]

+1 for a DCHECK.

If we hit the DCHECK elsewhere I think those could be real bugs to fix.

For more context, the particular issue here was libfuzz tests leaking around
20MB of histogram data.

This was coming from repeatedly instantiating histograms by
CookieMonster::InitializeHistograms, which was getting reached once per fuzzed
input:

https://code.google.com/p/chromium/codesearch#chromium/src/net/cookies/cookie...

[commit-bot: I haz the power, 2016-05-02 20:08:43]

Description was changed from

==========
net:  Prevent fuzzers from leaking on each iteration.

While the UMA macros use statics to avoid leaking, code that directly
invokes base::Histogram::FactoryGet will keep on creating new objects
on each iteration, unless the StatisticsRecorder has been initialized.
This CL modifies net's Fuzzer infrastructure to ensure it is
initialized.

BUG=602440
==========

to

==========
net:  Prevent fuzzers from leaking on each iteration.

While the UMA macros use statics to avoid leaking, code that directly
invokes base::Histogram::FactoryGet will keep on creating new objects
on each iteration, unless the StatisticsRecorder has been initialized.
This CL modifies net's Fuzzer infrastructure to ensure it is
initialized.

BUG=602440
==========

[commit-bot: I haz the power, 2016-05-02 20:08:44]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-02 20:11:01]

Description was changed from

==========
net:  Prevent fuzzers from leaking on each iteration.

While the UMA macros use statics to avoid leaking, code that directly
invokes base::Histogram::FactoryGet will keep on creating new objects
on each iteration, unless the StatisticsRecorder has been initialized.
This CL modifies net's Fuzzer infrastructure to ensure it is
initialized.

BUG=602440
==========

to

==========
net:  Prevent fuzzers from leaking on each iteration.

While the UMA macros use statics to avoid leaking, code that directly
invokes base::Histogram::FactoryGet will keep on creating new objects
on each iteration, unless the StatisticsRecorder has been initialized.
This CL modifies net's Fuzzer infrastructure to ensure it is
initialized.

BUG=602440

Committed: https://crrev.com/0960f7561368543f46cc126d64820307728faa2f
Cr-Commit-Position: refs/heads/master@{#391039}
==========

[commit-bot: I haz the power, 2016-05-02 20:11:02]

Patchset 1 (id:??) landed as
https://crrev.com/0960f7561368543f46cc126d64820307728faa2f
Cr-Commit-Position: refs/heads/master@{#391039}