Closed Bug 1105232 Opened 8 years ago Closed 8 years ago

Assertion failure running v8-v5/check-splay.js test with compacting GC

Categories

(Core :: JavaScript: GC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jonco, Assigned: jonco)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

When running the v8-v5/check-splay.js jit test with h4writer's tracelogger patches applied I see:

Assertion failure: overlay->isForwarded(), at /home/jon/clone/compacting/js/src/jsgc.h:1312
This is a race condition between multiple threads attempting to update a copy-on-write array object's owner object, caused by the fact that we update cells in parallel now.

This actually doesn't matter if this happens because we will always write the same updated pointer for the owner, so we just need to stop this triggering the assertion.
Attachment #8529074 - Flags: review?(terrence)
Attachment #8529074 - Flags: review?(terrence) → review+
As per dicussion with Waldo on irc, the compiler is allowed to tear the owner value so this code is still wrong.  Even with that fixed it's very hard to determine whether this is actually safe on all memory architectures.  So we're going to try and make updating the owner pointer for COW arrays use atomic operations here.
Keywords: leave-open
A better idea is to remove the race condition entirely, which is what this patch does.

We update the elements to owner pointer for copy on write objects which own their elements when we relocate the object in the first place.  This takes place on the main thread with no other threads running at the same time.
Attachment #8532041 - Flags: review?(terrence)
Comment on attachment 8532041 [details] [diff] [review]
bug1105232-fixup-cow-owner-earlier

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

Makes sense.
Attachment #8532041 - Flags: review?(terrence) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Keywords: leave-open
Resolution: --- → FIXED
Depends on: 1132171
You need to log in before you can comment on or make changes to this bug.