Closed Bug 2067675 Opened 1 month ago Closed 1 month ago

Hold mMutex while reading mCompileResult in WasmCompileTask::StealResult (Coverity CID 1700566)

Categories

(Core :: JavaScript: WebAssembly, task, P1)

task

Tracking

()

RESOLVED FIXED
157 Branch
Tracking Status
firefox157 --- fixed

People

(Reporter: Sylvestre, Assigned: Sylvestre)

References

(Blocks 1 open bug)

Details

Attachments

(1 external file)

Coverity report:

** CID 1700566:       Concurrent data access violations  (MISSING_LOCK)
/dom/script/ScriptLoader.cpp: 2620           in mozilla::dom::WasmCompileTask::StealResult(JSContext *, JS::MutableHandle<JSObject *>)()

2614       return TaskResult::Complete;
2615     }
2616
2617     bool WasmCompileTask::StealResult(JSContext* aCx,
2618                                       JS::MutableHandle<JSObject*> aModuleOut) {
2619       JS::Rooted<JSObject*> wasmModuleObject(aCx);
>>>     CID 1700566:         Concurrent data access violations  (MISSING_LOCK)
>>>     Accessing "this->mCompileResult" without holding lock "mozilla::detail::MutexImpl.mMutex". Elsewhere, "mozilla::dom::WasmCompileTask.mCompileResult" is written to with "MutexImpl.mMutex" held 1 out of 1 times.
2620       if (!JS::FinishCompileForESM(aCx, *mCompileArgs, mCompileResult,
2621                                    &wasmModuleObject)) {
2622         return false;
2623       }
2624
2625       aModuleOut.set(JS::CreateWasmSourcePhaseModule(aCx, wasmModuleObject));
```)

mCompileResult is written by Run() with mMutex held, and read here without it.
The read happens on the main thread once the task has completed, so there is
no actual race, but taking the lock is uncontended here and silences Coverity
(CID 1700566).

Assignee: nobody → sledru
Status: NEW → ASSIGNED
Component: DOM: Core & HTML → JavaScript: WebAssembly
Severity: -- → S3
Priority: -- → P1
Pushed by sledru@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/655f079fa58b https://hg.mozilla.org/integration/autoland/rev/904341b854da Hold mMutex while reading mCompileResult in WasmCompileTask::StealResult r=dminor
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 157 Branch
QA Whiteboard: [qa-triage-done-c158/b157]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: