Closed
Bug 1339051
Opened 7 years ago
Closed 7 years ago
Data race in iJIT_GetNewMethodID js/src/vtune/jitprofiling.c
Categories
(Core :: JavaScript Engine: JIT, defect, P1)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: jseward, Assigned: sstangl)
Details
Attachments
(2 files, 1 obsolete file)
30.01 KB,
text/plain
|
Details | |
6.91 KB,
patch
|
h4writer
:
review+
|
Details | Diff | Splinter Review |
I think this might be fallout from bug 1332466. TSan reports races as shown in the attachment, when starting the browser.
Assignee | ||
Comment 1•7 years ago
|
||
Ah, good catch. I should have checked: VTune internals are not threadsafe. This patch moves the DLL/SO load to initialization time and protects the VTune internals with a Big Lock.
Attachment #8837815 -
Flags: review?(hv1989)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → sstangl
Assignee | ||
Comment 2•7 years ago
|
||
Fixes small logic error. My VTune trial license expired and so I can't test it, but I'll get in contact with Intel to fix that.
Attachment #8837815 -
Attachment is obsolete: true
Attachment #8837815 -
Flags: review?(hv1989)
Attachment #8837825 -
Flags: review?(hv1989)
Comment 3•7 years ago
|
||
Comment on attachment 8837825 [details] [diff] [review] 0001-Bug-1339051-Make-VTune-JIT-integration-threadsafe.-r.patch Review of attachment 8837825 [details] [diff] [review]: ----------------------------------------------------------------- 1) Please remove all double newlines to one newline in VTuneWrapper.cpp 2) I think it would be better to wrap the Locks in VTuneWrapper.cpp in {} to make sure they are only initialized at that place. Else they can be hoisted to the top of the function IIRC. => as a side-note: Wouldn't it be better to create a wrapper function for iJIT_NotifyEvent that does locking and calls iJIT_NotifyEvent ??
Attachment #8837825 -
Flags: review?(hv1989) → review+
Updated•7 years ago
|
Priority: -- → P1
Pushed by sstangl@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/12d7b6083d81 Make VTune JIT integration threadsafe. r=h4writer
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/12d7b6083d81
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•