443878 - XPConnect doesn't always suspend requests when switching contexts

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch

While working on bug 437152 I hit some deadlocks where XPConnect wasn't properly suspending requests on a context that was already on the stack when it switched to a new context. Patch attached.

Comment 1

[Johnny Stenback (:jst)]

Comment on attachment 328301 [details] [diff] [review]
Patch

     // We need to use the safe context for this thread because we don't want
     // to parent the new (and cached forever!) function object to the current
     // JSContext's global object. That would be bad!
 
-    JSContext* cx = ccx.GetSafeJSContext();
-    if(!cx)
-        return JS_FALSE;
-

Move the comment with this change too.

+    {
+      // Switching
+      AutoJSSuspendRequest req(ccx);
+
+      JSAutoRequest ar(cx);
+  
+      fun = JS_NewFunction(cx, callback, argc, flags, nsnull, memberName);
+    }

Do we need to worry about the case where ccx == cx here? I guess it's not the end of the world to end a request only to begin another one, and then the reverse too. It's certainly possible to hit that case, but I don't know for sure how common of a case that is.

r+sr=jst with that figured out one way or another.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1.1

Moved the comment and fixed the case where cx and ccx are the same. I'm not using the auto helpers anymore, think that's ok?

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Pushed changeset be03f86900e1.