Closed Bug 621072 Opened 9 years ago Closed 9 years ago

TM: Traces shouldn't bake in interruptFlags

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: billm, Assigned: billm)

References

(Blocks 1 open bug)

Details

(Whiteboard: [cib-workers] [fixed-in-tracemonkey])

Attachments

(1 file, 1 obsolete file)

Since web workers can move from one thread to another, and the old thread may disappear, it isn't valid to bake &JS_THREAD_DATA(cx)->interruptFlags into traces.
Assignee: general → wmccloskey
blocking2.0: --- → ?
Attached patch patch (obsolete) — Splinter Review
This is the simplest way to fix this.
Attachment #499433 - Flags: review?(igor)
That's an empty patch :)
Attached patch updated patchSplinter Review
It's a simple patch, but not that simple. I messed up when I split two changes apart.
Attachment #499433 - Attachment is obsolete: true
Attachment #499437 - Flags: review?(igor)
Attachment #499433 - Flags: review?(igor)
I thought this was going to require an extra load per check, glad to see it doesn't!
blocking2.0: ? → betaN+
Whiteboard: [cib-workers]
Blocks: 621140
Comment on attachment 499437 [details] [diff] [review]
updated patch

>From: Bill McCloskey <wmccloskey@mozilla.com>
>try: -p linux,linux64,win32
>
>diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp
>--- a/js/src/jstracer.cpp
>+++ b/js/src/jstracer.cpp
>@@ -2345,17 +2345,22 @@ TraceRecorder::TraceRecorder(JSContext* 
> 
>     if (fragment == fragment->root) {
>         /*
>          * We poll the operation callback request flag. It is updated asynchronously whenever
>          * the callback is to be invoked. We can use w.nameImmpNonGC here as JIT-ed code is per
>          * thread and cannot outlive the corresponding JSThreadData.
>          */
>         w.comment("begin-interruptFlags-check");
>-        LIns* flagptr = w.nameImmpNonGC((void *) &JS_THREAD_DATA(cx)->interruptFlags);
>+#ifdef JS_THREADSAFE
>+        void *interrupt = (void*) &cx->runtime->interruptCounter;
>+#else
>+        void *interrupt = (void*) &JS_THREAD_DATA(cx)->interruptFlags;
>+#endif

Add fixme bug 621140 about per-compartment flag here.
Attachment #499437 - Flags: review?(igor) → review+
http://hg.mozilla.org/tracemonkey/rev/6255a0255dc2
Whiteboard: [cib-workers] → [cib-workers] [fixed-in-tracemonkey]
http://hg.mozilla.org/mozilla-central/rev/c83c130ce23f
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.