Lend LSan a hand and clear out singleton static persistents first.

Lend LSan a hand and clear out singleton static persistents first.

To prepare for LSan's determination of leaking objects, the main thread
will GC its heap, finalizing objects with references to off-heap ones.
This must also encompass heap objects that are held onto by static
persistents, so we arrange for all of those singletons to be released.

Releasing the static persistents leaves them in an invalid, null state,
so any object that becomes garbage by the release of a static persistent
must not attempt to access any of these singletons while being subsequently
finalized. This is a restriction that's hard to work around should it
turn out to strike, so instead the singletons are attempted cleared
first. That will make the objects they retain eligible for garbage
collection (and finalization) earlier, avoiding the problem of touching
singletons backed by (now-released) persistent references.

R=
BUG=677684

[sof, 2017-01-02 08:28:16]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-02 08:28:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2017-01-02 09:03:43]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2017-01-02 09:03:44]

please take a look.

Somewhat of a blind fix, as I've not been able to reproduce.

[haraken, 2017-01-02 09:26:44]

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

[haraken, 2017-01-02 09:26:44]

https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:340:
clearStaticPersistentNodes();

Hmm, I'm not quite sure if this helps.

In my understanding, LSan on a renderer process runs before blink::shutdown
getting called. See
https://cs.chromium.org/chromium/src/content/renderer/renderer_main.cc?sq=pac...

__lsan_do_leak_check() is called before RenderProcessImpl's object gets
destructed, in which blink::shutdown is called.

[sof, 2017-01-02 09:37:17]

https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:340:
clearStaticPersistentNodes();
On 2017/01/02 09:26:44, haraken wrote:
> 
> Hmm, I'm not quite sure if this helps.
> 
> In my understanding, LSan on a renderer process runs before blink::shutdown
> getting called. See
>
https://cs.chromium.org/chromium/src/content/renderer/renderer_main.cc?sq=pac...
> 
> __lsan_do_leak_check() is called before RenderProcessImpl's object gets
> destructed, in which blink::shutdown is called.
> 

Hmm, that doesn't make a lot of sense, given the trouble that LSan has given us
in the past by not clearing up and GCing first.

[commit-bot: I haz the power, 2017-01-02 09:53:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-02 09:53:49]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-01-02 10:44:58]

On 2017/01/02 09:37:17, sof wrote:
>
https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
> File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):
> 
>
https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
> third_party/WebKit/Source/platform/heap/ThreadState.cpp:340:
> clearStaticPersistentNodes();
> On 2017/01/02 09:26:44, haraken wrote:
> > 
> > Hmm, I'm not quite sure if this helps.
> > 
> > In my understanding, LSan on a renderer process runs before blink::shutdown
> > getting called. See
> >
>
https://cs.chromium.org/chromium/src/content/renderer/renderer_main.cc?sq=pac...
> > 
> > __lsan_do_leak_check() is called before RenderProcessImpl's object gets
> > destructed, in which blink::shutdown is called.
> > 
> 
> Hmm, that doesn't make a lot of sense, given the trouble that LSan has given
us
> in the past by not clearing up and GCing first.

Yeah, I was thinking that LSan is running after blink::shutdown being called.
However, while working on https://codereview.chromium.org/2309153002, I noticed
that that is not the case (at least in a normal renderer process; the situation
might be different in some tests).

[sof, 2017-01-02 13:34:19]

On 2017/01/02 10:44:58, haraken wrote:
> On 2017/01/02 09:37:17, sof wrote:
> >
>
https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
> > File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
> > third_party/WebKit/Source/platform/heap/ThreadState.cpp:340:
> > clearStaticPersistentNodes();
> > On 2017/01/02 09:26:44, haraken wrote:
> > > 
> > > Hmm, I'm not quite sure if this helps.
> > > 
> > > In my understanding, LSan on a renderer process runs before
blink::shutdown
> > > getting called. See
> > >
> >
>
https://cs.chromium.org/chromium/src/content/renderer/renderer_main.cc?sq=pac...
> > > 
> > > __lsan_do_leak_check() is called before RenderProcessImpl's object gets
> > > destructed, in which blink::shutdown is called.
> > > 
> > 
> > Hmm, that doesn't make a lot of sense, given the trouble that LSan has given
> us
> > in the past by not clearing up and GCing first.
> 
> Yeah, I was thinking that LSan is running after blink::shutdown being called.
> However, while working on https://codereview.chromium.org/2309153002, I
noticed
> that that is not the case (at least in a normal renderer process; the
situation
> might be different in some tests).

Those explicit LSan leak checks have been there for some time, before we added
the extra GCs on the Oilpan side.

We can try to take them out and see if anything breaks.

[sof, 2017-01-04 08:52:28]

On 2017/01/02 13:34:19, sof wrote:
> On 2017/01/02 10:44:58, haraken wrote:
> > On 2017/01/02 09:37:17, sof wrote:
> > >
> >
>
https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
> > > File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2602263002/diff/1/third_party/WebKit/Source/p...
> > > third_party/WebKit/Source/platform/heap/ThreadState.cpp:340:
> > > clearStaticPersistentNodes();
> > > On 2017/01/02 09:26:44, haraken wrote:
> > > > 
> > > > Hmm, I'm not quite sure if this helps.
> > > > 
> > > > In my understanding, LSan on a renderer process runs before
> blink::shutdown
> > > > getting called. See
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/content/renderer/renderer_main.cc?sq=pac...
> > > > 
> > > > __lsan_do_leak_check() is called before RenderProcessImpl's object gets
> > > > destructed, in which blink::shutdown is called.
> > > > 
> > > 
> > > Hmm, that doesn't make a lot of sense, given the trouble that LSan has
given
> > us
> > > in the past by not clearing up and GCing first.
> > 
> > Yeah, I was thinking that LSan is running after blink::shutdown being
called.
> > However, while working on https://codereview.chromium.org/2309153002, I
> noticed
> > that that is not the case (at least in a normal renderer process; the
> situation
> > might be different in some tests).
> 
> Those explicit LSan leak checks have been there for some time, before we added
> the extra GCs on the Oilpan side.
> 
> We can try to take them out and see if anything breaks.

https://codereview.chromium.org/2604413002 appears to have settled in just fine;
good. Closing this.