1342473 - PersistentRooted races on runtime with shell workers

Status: RESOLVED WORKSFORME

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

Description

[Steve Fink [:sfink] [:s:]]

JS::AddPersistentRoot(JS::RootingContext* cx, RootKind kind, PersistentRooted<void*>* root)
{
    static_cast<JSContext*>(cx)->runtime()->heapRoots.ref()[kind].insertBack(root);
}

This races, since it looks like we run shell workers concurrently.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: tsan.txt

Comment 2

[Jon Coppeard (:jonco)]

(In reply to Steve Fink [:sfink] [:s:] from comment #0)
According to the log, the race is between two cooperatively scheduled worker threads.

JSRuntime::heapRoots is wrapped in js::ActiveThreadData<> which assert CurrentThreadCanAccessRuntime() on access, so adding to a this list is checked to be safe.  However the linked list element's destructor removes the element from the list without performing this check.

I'm not sure how cooperative scheduling of threads works.  But to start with we should audit any code that uses a protected linked list like this and assert that we can access the list in the element object's destructor.

Comment 3

[Jan de Mooij [:jandem]]

Cooperative scheduling was removed + we no longer see this in automation.