Closed Bug 1458839 Opened 2 years ago Closed 2 years ago

Refactor GCParallelTask a little and improve state assertions

Categories

(Core :: JavaScript: GC, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: jonco, Assigned: jonco)

Details

Attachments

(1 file)

We should add assertions to state transitions for GCParallelTask to check the original state is as expected and that we hold the helper thread lock when we do this.
Here's a patch to improve the assertions around GCParallelTask state management.

I removed the argument to the cancel() method which was always passed GCParallelTask::CancelAndWait.

I found one actual bug - the cancel field wasn't being initialised in the first constructor.  This is only used in background alloc and decommit tasks though, which don't have to run to completion so it shouldn't have caused any actual problems.
Attachment #8972852 - Flags: review?(jdemooij)
Comment on attachment 8972852 [details] [diff] [review]
bug1458839-parallel-task-assertions

Review of attachment 8972852 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.
Attachment #8972852 - Flags: review?(jdemooij) → review+
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a35392007e5d
Improve state assertions in GCParallelTask r=jandem
https://hg.mozilla.org/mozilla-central/rev/a35392007e5d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.