Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(117)

Issue 1171463006: Make Timer and CancellableTaskFactory embeddeable in lazily swept objects. (Closed)

Created:
5 years, 6 months ago by sof
Modified:
5 years, 6 months ago
Reviewers:
oilpan-reviews, haraken
CC:
blink-reviews, scheduler-bugs_chromium.org
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Make Timer and CancellableTaskFactory embeddeable in lazily swept objects. As CancellableTaskFactory is a part object, it might be part of an object that's on the Oilpan heap. TimerBase's task factory being the main example. With ASan and Oilpan, heap objects will be poisoned prior to the Oilpan GC sweep stage. Hence, should a cancellable task be attempted run while the owning object is poisoned, poisoned memory will be accessed. Allow such accesses by explicitly unpoisoning the object first. This is considered acceptable as the memory region is within a larger object that still is poisoned, hence ASan will continue to be effective. For Timer, reposition its is-alive check over the object so as to make it compatible with the timer tasks that now are run. R=haraken BUG=482388 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196674

Patch Set 1 #

Total comments: 8

Patch Set 2 : mark ScriptRunner's CancellableTaskFactory as needing unpoisoning #

Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -15 lines) Patch
M Source/core/dom/ScriptRunner.cpp View 1 1 chunk +3 lines, -0 lines 0 comments Download
M Source/platform/Timer.h View 5 chunks +26 lines, -11 lines 0 comments Download
M Source/platform/Timer.cpp View 2 chunks +4 lines, -1 line 0 comments Download
M Source/platform/scheduler/CancellableTaskFactory.h View 4 chunks +16 lines, -0 lines 0 comments Download
M Source/platform/scheduler/CancellableTaskFactory.cpp View 1 chunk +8 lines, -3 lines 0 comments Download

Messages

Total messages: 24 (4 generated)
sof
please take a look. https://codereview.chromium.org/1134523002 relanded last night, address (expected) ASan problems, http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Oilpan%20ASAN/builds/1741
5 years, 6 months ago (2015-06-06 07:45:05 UTC) #2
haraken
https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp File Source/platform/scheduler/CancellableTaskFactory.cpp (right): https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp#newcode30 Source/platform/scheduler/CancellableTaskFactory.cpp:30: ASAN_UNPOISON_MEMORY_REGION(reinterpret_cast<unsigned char*>(taskFactory), sizeof(CancellableTaskFactory)); Just help me understand: Where can ...
5 years, 6 months ago (2015-06-06 15:25:20 UTC) #4
sof
https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp File Source/platform/scheduler/CancellableTaskFactory.cpp (right): https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp#newcode30 Source/platform/scheduler/CancellableTaskFactory.cpp:30: ASAN_UNPOISON_MEMORY_REGION(reinterpret_cast<unsigned char*>(taskFactory), sizeof(CancellableTaskFactory)); On 2015/06/06 15:25:20, haraken wrote: > ...
5 years, 6 months ago (2015-06-06 15:46:11 UTC) #5
haraken
https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp File Source/platform/scheduler/CancellableTaskFactory.cpp (right): https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp#newcode30 Source/platform/scheduler/CancellableTaskFactory.cpp:30: ASAN_UNPOISON_MEMORY_REGION(reinterpret_cast<unsigned char*>(taskFactory), sizeof(CancellableTaskFactory)); On 2015/06/06 15:46:11, sof wrote: > ...
5 years, 6 months ago (2015-06-07 16:40:46 UTC) #6
sof
https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp File Source/platform/scheduler/CancellableTaskFactory.cpp (right): https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp#newcode30 Source/platform/scheduler/CancellableTaskFactory.cpp:30: ASAN_UNPOISON_MEMORY_REGION(reinterpret_cast<unsigned char*>(taskFactory), sizeof(CancellableTaskFactory)); On 2015/06/07 16:40:46, haraken wrote: > ...
5 years, 6 months ago (2015-06-07 17:23:05 UTC) #7
haraken
https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp File Source/platform/scheduler/CancellableTaskFactory.cpp (right): https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp#newcode30 Source/platform/scheduler/CancellableTaskFactory.cpp:30: ASAN_UNPOISON_MEMORY_REGION(reinterpret_cast<unsigned char*>(taskFactory), sizeof(CancellableTaskFactory)); On 2015/06/07 17:23:05, sof wrote: > ...
5 years, 6 months ago (2015-06-07 23:46:24 UTC) #8
sof
On 2015/06/07 23:46:24, haraken wrote: > https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp > File Source/platform/scheduler/CancellableTaskFactory.cpp (right): > > https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp#newcode30 > ...
5 years, 6 months ago (2015-06-08 05:12:23 UTC) #9
haraken
On 2015/06/08 05:12:23, sof wrote: > On 2015/06/07 23:46:24, haraken wrote: > > > https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp ...
5 years, 6 months ago (2015-06-08 05:18:37 UTC) #10
sof
On 2015/06/08 05:18:37, haraken wrote: > On 2015/06/08 05:12:23, sof wrote: > > On 2015/06/07 ...
5 years, 6 months ago (2015-06-08 05:33:51 UTC) #11
haraken
On 2015/06/08 05:33:51, sof wrote: > On 2015/06/08 05:18:37, haraken wrote: > > On 2015/06/08 ...
5 years, 6 months ago (2015-06-08 05:39:50 UTC) #12
sof
On 2015/06/08 05:39:50, haraken wrote: > On 2015/06/08 05:33:51, sof wrote: > > On 2015/06/08 ...
5 years, 6 months ago (2015-06-08 05:52:59 UTC) #13
haraken
Sorry! I now got your point, LGTM. (The CancellableTaskFactory in Timer was not grepped in ...
5 years, 6 months ago (2015-06-08 06:51:53 UTC) #14
sof
https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp File Source/platform/scheduler/CancellableTaskFactory.cpp (right): https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp#newcode29 Source/platform/scheduler/CancellableTaskFactory.cpp:29: if (taskFactory->m_unpoisonBeforeUpdate) On 2015/06/08 06:51:52, haraken wrote: > > ...
5 years, 6 months ago (2015-06-08 07:04:37 UTC) #15
haraken
https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp File Source/platform/scheduler/CancellableTaskFactory.cpp (right): https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp#newcode29 Source/platform/scheduler/CancellableTaskFactory.cpp:29: if (taskFactory->m_unpoisonBeforeUpdate) On 2015/06/08 07:04:37, sof wrote: > On ...
5 years, 6 months ago (2015-06-08 07:11:10 UTC) #16
sof
On 2015/06/08 07:11:10, haraken wrote: > https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp > File Source/platform/scheduler/CancellableTaskFactory.cpp (right): > > https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp#newcode29 > ...
5 years, 6 months ago (2015-06-08 09:27:03 UTC) #17
haraken
On 2015/06/08 09:27:03, sof wrote: > On 2015/06/08 07:11:10, haraken wrote: > > > https://codereview.chromium.org/1171463006/diff/1/Source/platform/scheduler/CancellableTaskFactory.cpp ...
5 years, 6 months ago (2015-06-08 10:57:49 UTC) #18
sof
On 2015/06/08 10:57:49, haraken wrote: > On 2015/06/08 09:27:03, sof wrote: > > On 2015/06/08 ...
5 years, 6 months ago (2015-06-08 11:21:24 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1171463006/20001
5 years, 6 months ago (2015-06-08 11:43:55 UTC) #22
haraken
On 2015/06/08 11:21:24, sof wrote: > On 2015/06/08 10:57:49, haraken wrote: > > On 2015/06/08 ...
5 years, 6 months ago (2015-06-08 11:50:05 UTC) #23
commit-bot: I haz the power
5 years, 6 months ago (2015-06-08 13:26:10 UTC) #24
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196674

Powered by Google App Engine
This is Rietveld 408576698