Invalidate cross-thread persistents on heap termination.

Invalidate cross-thread persistents on heap termination.

When a thread is detached from Oilpan and its ThreadState is
finalized, arrange for any CrossThreadPersistent<>s pointing
into one of its heaps to be cleared out. Not doing so risks
dangling pointers to be followed upon GC or by anyone else
still keeping these CrossThreadPersistent<>s alive.

The only operation that other threads are allowed over
CrossThreadPersistent<> (CTP) once the ThreadState has been
destructed, is to destruct the CTP.

R=haraken
BUG=515432
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199891

[sof, 2015-08-02 16:29:56]

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

[sof, 2015-08-02 16:29:57]

please take a look.

If you accept the premise that CTPs can live longer than the objects they refer
to (due to thread heap shutdown), then something like this seems in order to
make that safe.

I haven't invested much time in optimizing the invalidation pass, for now.

[haraken, 2015-08-03 00:45:25]

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

[haraken, 2015-08-03 00:45:25]

Thanks for working on this! Mostly LG.

(I think this will be a good step to make our heaps per-thread. I need to write
a design doc though...)

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Pe...
File Source/platform/heap/PersistentNode.cpp (right):

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Pe...
Source/platform/heap/PersistentNode.cpp:68: // This to allow an Oilpan thread to
shut down and terminate

This is to allow

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Pe...
Source/platform/heap/PersistentNode.cpp:129: // into the heaps of each
ThreadState & use that count to terminate traversal.)

Given that the number of CrossThreadPersistents should be limited, I'm not
worried about the overhead very much.

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Pe...
File Source/platform/heap/PersistentNode.h (right):

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Pe...
Source/platform/heap/PersistentNode.h:88: m_self =
reinterpret_cast<Address>(reinterpret_cast<uintptr_t>(m_self) | 0x1);

Or I'd prefer just clearing the pointer. The semantics will be:

- CrossThreadPersistent can outlive the thread that has allocated the pointing
object. The outlived CrossThreadPersistent is cleared out when the owner thread
gets detached.

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Th...
File Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Th...
Source/platform/heap/ThreadState.cpp:244: prepareHeapForTermination();

Nit: I'd rename this to prepareForThreadStateTermination().

[sof, 2015-08-03 08:30:04]

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Pe...
File Source/platform/heap/PersistentNode.cpp (right):

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Pe...
Source/platform/heap/PersistentNode.cpp:129: // into the heaps of each
ThreadState & use that count to terminate traversal.)
On 2015/08/03 00:45:25, haraken wrote:
> 
> Given that the number of CrossThreadPersistents should be limited, I'm not
> worried about the overhead very much.

I'd like to revisit and address, but thread local shutdowns aren't that
plentiful to boot.

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Pe...
File Source/platform/heap/PersistentNode.h (right):

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Pe...
Source/platform/heap/PersistentNode.h:88: m_self =
reinterpret_cast<Address>(reinterpret_cast<uintptr_t>(m_self) | 0x1);
On 2015/08/03 00:45:25, haraken wrote:
> 
> Or I'd prefer just clearing the pointer. The semantics will be:
> 
> - CrossThreadPersistent can outlive the thread that has allocated the pointing
> object. The outlived CrossThreadPersistent is cleared out when the owner
thread
> gets detached.
> 

Can do that instead, that is, clear out the underlying raw pointer of the m_self
here.

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Th...
File Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/1265103003/diff/20001/Source/platform/heap/Th...
Source/platform/heap/ThreadState.cpp:244: prepareHeapForTermination();
On 2015/08/03 00:45:25, haraken wrote:
> 
> Nit: I'd rename this to prepareForThreadStateTermination().

Done.

[haraken, 2015-08-03 09:34:53]

LGTM

[sof, 2015-08-03 09:57:54]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-08-03 09:57:58]

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

[sof, 2015-08-03 09:58:04]

The CQ bit was unchecked by sigbjornf@opera.com

[sof, 2015-08-03 09:58:15]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-08-03 09:58:35]

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

[commit-bot: I haz the power, 2015-08-03 10:40:49]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199891