660783 - dom workers should not assert about non-main thread during GC marking

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

For parallel marking from bug 638660 we need to make sure that all JSClass::trace hooks can be called from multiple threads and tolerate parallel execution for the same object.

Yet the hook implementation in dpm/src/treads insists that the calling thread is the main thread. AFAICS the assert can be removed as the GC graph traversal in dom/src/threads is stateless.

Comment 1

[Igor Bukanov]

Attachment: v1

The patch removes the assert under the assumption that nsDOMWorkerMessageHandler::Trace is stateless.

Comment 2

[Igor Bukanov]

Ben: nsDOMWorker::Trace only mark its collection if the worker is not cancelled. As this mayleaves a live object with finalized GC things reachable via collections, it require care when accessing those. So why not to call nsDOMWorkerMessageHandler::Trace there unconditionally?

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #2)
> Ben: nsDOMWorker::Trace only mark its collection if the worker is not
> cancelled. As this mayleaves a live object with finalized GC things
> reachable via collections, it require care when accessing those. So why not
> to call nsDOMWorkerMessageHandler::Trace there unconditionally?

I'm not sure... It might have been to prevent some type of cyclical leak in the past (all of these objects are very cyclical). In any case it's safe to have finalized gcthings in the collection as long as the canceled flag is set since we will never attempt to use them once canceled.

All of this code has changed in bug 649537 (and Trace is threadsafe there) so I'd rather not change anything here but remove the assertion.

Comment 4

[Igor Bukanov]

http://hg.mozilla.org/tracemonkey/rev/a4a11ea5646c

Comment 5

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/a4a11ea5646c