Closed Bug 410323 Opened 14 years ago Closed 14 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?
Perhaps fixed by bug 402966?
(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: 14 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: 14 years ago14 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.