Lock CrossThreadPersistentRegion while poisoning HeapPages

Lock CrossThreadPersistentRegion while poisoning HeapPages

When CrossThreadPersistentRegion::prepareForThreadStateTermination or CrossThreadPersistentRegion::tracePersistentNodes is called, we iterate over all CrossThreadPersistents.
While that is happening in one thread, another thread was calling ThreadState::poisonAllHeaps(), causing poisoning to happen simultaneously.

BUG=660760
Committed: https://crrev.com/e83436f136fef95c74f113f456327edf33a4339b
Cr-Commit-Position: refs/heads/master@{#429842}

[keishi, 2016-11-02 15:03:46]

keishi@chromium.org changed reviewers:
+ haraken@chromium.org, oilpan-reviews@chromium.org

[keishi, 2016-11-02 15:03:46]

PTAL

[haraken, 2016-11-02 15:07:45]

Maybe the real problem is that the poisoning is not happening during the
stop-the-world phase? If so, a better fix would be to move the poisoning
operation to postGC().

[keishi, 2016-11-04 07:49:34]

Patchset #2 (id:20001) has been deleted

[keishi, 2016-11-04 07:59:33]

On 2016/11/02 15:07:45, haraken wrote:
> Maybe the real problem is that the poisoning is not happening during the
> stop-the-world phase? If so, a better fix would be to move the poisoning
> operation to postGC().

threadLocalWeakProcessing and PreFinalizers need to access unmarked objects
(those happen in preSweep)
If we unpoison and poison each object like we do for sweep, we need to call
unpoisonCrossThreadPersistents() after we re-poison every object.
I'm not sure if this is worth the trouble.
We don't need to call unpoisonCrossThreadPersistents() for every unmarked object
for sweep, because the CrossThreadPersistents in that object will be destroyed
during sweep.

Also, are eagerly finalized object are allowed to touch unmarked objects? (Is
this why poisonEagerArena is separate?)

[haraken, 2016-11-04 08:10:50]

Discussed offline. LGTM.

[keishi, 2016-11-04 08:20:02]

The CQ bit was checked by keishi@chromium.org

[commit-bot: I haz the power, 2016-11-04 08:20:23]

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

[commit-bot: I haz the power, 2016-11-04 09:50:34]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-04 09:52:23]

Description was changed from

==========
Lock CrossThreadPersistentRegion while poisoning HeapPages

When CrossThreadPersistentRegion::prepareForThreadStateTermination or
CrossThreadPersistentRegion::tracePersistentNodes is called, we iterate over all
CrossThreadPersistents.
While that is happening in one thread, another thread was calling
ThreadState::poisonAllHeaps(), causing poisoning to happen simultaneously.

BUG=660760
==========

to

==========
Lock CrossThreadPersistentRegion while poisoning HeapPages

When CrossThreadPersistentRegion::prepareForThreadStateTermination or
CrossThreadPersistentRegion::tracePersistentNodes is called, we iterate over all
CrossThreadPersistents.
While that is happening in one thread, another thread was calling
ThreadState::poisonAllHeaps(), causing poisoning to happen simultaneously.

BUG=660760

Committed: https://crrev.com/e83436f136fef95c74f113f456327edf33a4339b
Cr-Commit-Position: refs/heads/master@{#429842}
==========

[commit-bot: I haz the power, 2016-11-04 09:52:23]

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