TSan: enabledTextIds data race

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks: 1 bug)

unspecified
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
The main thread calls Debugger::setupTraceLogger -> TraceLogEnableTextId

A background compilation thread does CodeGeneratorShared::emitTracelogScript -> TraceLogTextIdEnabled.

Should we cancel compilations before we change TraceLogger settings?
Flags: needinfo?(hv1989)
we call ReleaseAllJITCode in TraceLoggerThreadState::enableTextId.
IIUC that should cancel all compilations currently happening, right?
Flags: needinfo?(hv1989)
(Assignee)

Comment 2

3 years ago
Created attachment 8764126 [details] [diff] [review]
Patch

(In reply to Hannes Verschore [:h4writer] from comment #1)
> we call ReleaseAllJITCode in TraceLoggerThreadState::enableTextId.
> IIUC that should cancel all compilations currently happening, right?

Ah yes, we just have to cancel before we change the arrays. This fixes the TSan failure.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8764126 - Flags: review?(hv1989)
Attachment #8764126 - Flags: review?(hv1989) → review+

Comment 3

3 years ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5dd168681220
Fix a TSan data race in TraceLogger enabledTextIds. r=h4writer

Comment 4

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5dd168681220
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.