Closed Bug 410323 Opened 13 years ago Closed 13 years ago

Assertion failure: !rt->gcRunning | ASSERTION: XPCWrappedNative::GetNewOrUsed called during GC

Categories

(Core :: JavaScript Engine, defect)

x86
All
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: bc, Assigned: mrbkap)

References

()

Details

(Keywords: crash, dogfood, regression)

Attachments

(2 files, 1 obsolete file)

Attached file stack
browser only.
sensitive since bug 391851 is.

steps to reproduce:

1. load url in debug trunk build
   if you have access to the office network, I can give you access to a mac intel 
   box, if you have access to the colo, I can give you access to a winxp box.
2. shutdown
3. crash

###!!! ASSERTION: XPCWrappedNative::GetNewOrUsed called during GC: '!Scope->GetRuntime()->GetThreadRunningGC()', file /work/mozilla/builds/1.9.0/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 295
Assertion failure: !rt->gcRunning, at /work/mozilla/builds/1.9.0/mozilla/js/src/jsgc.c:1336
Trace/breakpoint trap

regressed sometime between approx midnight 12/27 PST and 1:30pm PST 12/30. I'll work on getting a better regression range unless someone else can point out the bug.
Flags: blocking1.9?
This was caused by bug 399587. Do we explicitly mark regressions when one of the bugs is sensitive and the other isn't?
(In reply to comment #2)
> Perhaps fixed by bug 402966?

Nope. :-(
Blocks: 399587
Giving to mrbkap per comment 1.

Jesse says this is making it impossible for him to test debug trunk -- that's bad.

/be
Assignee: general → mrbkap
Flags: blocking1.9? → blocking1.9+
Keywords: dogfood
I hit this when I load https://mail.google.com/mail/ and also at other times.
It's silly for a regression this common to be security-sensitive, and I don't see anything in this bug that looks secret.
Group: security
This is causing orange on a Windows Tinderbox:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1199406780.1199412571.21537.gz&fulltext=1
Severity: normal → blocker
fixed by backing out bug 399587 comment 16
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Not "fixed," wallpapered over. Reopening to find a real fix.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Severity: blocker → critical
Status: REOPENED → ASSIGNED
Keywords: testcase
Attached patch Fix (obsolete) — Splinter Review
The fix here is to not call into js_LookupProperty (which calls resolve hooks) in a function that is called from GC. Furthermore, I don't think this code wants resolve hooks to run: it only needs to change the sprop properties if the property already exists, not if the property has been newly created by such a hook. This fix makes us use the scope directly to see if the object has the property. Also note that you're not allowed to set a watch point on a non-native object (as enforced by JS_SetWatchPoint).
Attachment #295406 - Flags: review?(brendan)
Comment on attachment 295406 [details] [diff] [review]
Fix

>+    JSScopeProperty *sprop, *sprop2;

Suggest wprop or longer (watched_sprop? nah, wprop) instead of sprop2.

>+        JS_LOCK_OBJ(cx, wp->object);
>+        scope = OBJ_SCOPE(wp->object);
>+        sprop2 = (scope->object == wp->object)
>+                 ? SCOPE_GET_PROPERTY(scope, sprop->id)
>+                 : NULL;
>+        JS_UNLOCK_SCOPE(cx, wp->object);

Oops -- need to pass scope to JS_UNLOCK_SCOPE.

>+        if (sprop2) {
>+            sprop = js_ChangeScopePropertyAttrs(cx, scope, sprop,
>+                                                0, sprop->attrs,
>+                                                sprop->getter,
>+                                                wp->setter);
>+            if (!sprop)
>+                ok = JS_FALSE;

Noting here that sprop2 is unused except as a boolean. Could avoid wprop in favor of a 'found' JSBool.

r/a=me with fixage.

/be
Attachment #295406 - Flags: review?(brendan)
Attachment #295406 - Flags: review+
Attachment #295406 - Flags: approval1.9+
Attachment #295406 - Attachment is obsolete: true
Attachment #295427 - Flags: review?(brendan)
Comment on attachment 295427 [details] [diff] [review]
Updated to comments

found can cuddle with ok on the first declaration line. r+a=me with that!

/be
Attachment #295427 - Flags: review?(brendan)
Attachment #295427 - Flags: review+
Attachment #295427 - Flags: approval1.9+
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Flags: in-litmus-
v
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.