336695 - Assertion failure: !OBJ_GET_PARENT(cx, obj), at jscntxt.c:882 when running installer

Status: RESOLVED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect, P1)

Description

[Andrew Schultz]

A debug-enabled seamonkey installer dies on linux with:
Assertion failure: !OBJ_GET_PARENT(cx, obj), at mozilla/js/src/jscntxt.c:882
which is the code from bug 304376

gdb is clueless, but valgrind sees this stack:
JS_Assert (in /tmp/xpi.2ZHiW3/bin/libmozjs.so)
js_SetClassObject (in /tmp/xpi.2ZHiW3/bin/libmozjs.so)
JS_InitClass (in /tmp/xpi.2ZHiW3/bin/libmozjs.so)
js_InitFunctionClass (in /tmp/xpi.2ZHiW3/bin/libmozjs.so)
js_InitFunctionAndObjectClasses (in /tmp/xpi.2ZHiW3/bin/libmozjs.so)
JS_InitStandardClasses (in /tmp/xpi.2ZHiW3/bin/libmozjs.so)
(within /tmp/xpi.2ZHiW3/bin/components/libxpinstall.so)
RunInstallOnThread (in /tmp/xpi.2ZHiW3/bin/components/libxpinstall.so)
RunInstall(nsInstallInfo*) (in /tmp/xpi.2ZHiW3/bin/components/libxpinstall.so)
nsSoftwareUpdate::RunNextInstall() (in /tmp/xpi.2ZHiW3/bin/components/libxpinstall.so)
nsSoftwareUpdate::InstallJar(nsIFile*, unsigned short const*, unsigned short const*, nsIPrincipal*, unsigned, nsIXPIListener*) (in /tmp/xpi.2ZHiW3/bin/components/libxpinstall.so)
XPI_Install (in /tmp/xpi.2ZHiW3/bin/libxpistub.so)

Comment 1

[Andrew Schultz]

*** Bug 337997 has been marked as a duplicate of this bug. ***

Comment 2

[Alex Vincent [:WeirdAl]]

I'm getting almost the same stack:

 	ntdll.dll!7c901230() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
 	js3250.dll!JS_Assert(const char * s=0x00e48964, const char * file=0x00e48944, int ln=2242)  Line 62	C
>	js3250.dll!js_SetClassObject(JSContext * cx=0x03716bf0, JSObject * obj=0x03777688, JSProtoKey key=JSProto_Function, JSObject * cobj=0x03777f30)  Line 2242 + 0x10b bytes	C
 	js3250.dll!JS_InitClass(JSContext * cx=0x03716bf0, JSObject * obj=0x03777688, JSObject * parent_proto=0x00000000, JSClass * clasp=0x00e33008, int (JSContext *, JSObject *, unsigned int, long *, long *)* constructor=0x00d76a80, unsigned int nargs=1, JSPropertySpec * ps=0x00e32ad0, JSFunctionSpec * fs=0x00e338b8, JSPropertySpec * static_ps=0x00000000, JSFunctionSpec * static_fs=0x00000000)  Line 2248 + 0x1b bytes	C
 	js3250.dll!js_InitFunctionClass(JSContext * cx=0x03716bf0, JSObject * obj=0x03777688)  Line 2037 + 0x29 bytes	C
 	js3250.dll!js_InitFunctionAndObjectClasses(JSContext * cx=0x03716bf0, JSObject * obj=0x03777688)  Line 1154 + 0xd bytes	C
 	js3250.dll!JS_InitStandardClasses(JSContext * cx=0x03716bf0, JSObject * obj=0x03777688)  Line 1198 + 0xd bytes	C
 	xpinstal.dll!SetupInstallContext(nsIZipReader * hZip=0x037703d0, nsIFile * jarFile=0x0376e290, const unsigned short * url=0x0376e640, const unsigned short * args=0x0376e538, unsigned int flags=0, nsIChromeRegistrySea * reg=0x0376e890, JSRuntime * rt=0x03773ec0, JSContext * * jsCX=0x04aaff40, JSObject * * jsGlob=0x04aaff34)  Line 407 + 0xe bytes	C++
 	xpinstal.dll!RunInstallOnThread(void * data=0x0376e5b8)  Line 538 + 0x47 bytes	C++
 	nspr4.dll!_PR_NativeRunThread(void * arg=0x0376ebf0)  Line 436 + 0xf bytes	C
 	nspr4.dll!pr_root(void * arg=0x0376ebf0)  Line 120 + 0xf bytes	C
 	msvcr80d.dll!102047c1() 	
 	ntdll.dll!7c910e91() 	
 	msvcr80d.dll!10204767() 	
 	kernel32.dll!7c80b50b() 	
 	ntdll.dll!7c910e91() 	
 	kernel32.dll!7c8399f3() 	

Comment 3

[Alex Vincent [:WeirdAl]]

Steps to reproduce:
(1) Start SeaMonkey
(2) Load URL field in browser
(3) Click "Install" on popup dialog

Expected results:

An empty directory is installed.

Actual results:
Crash.

Comment 4

[Alex Vincent [:WeirdAl]]

I've also seen this on Firefox trunk:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060529 Minefield/3.0a1

Comment 5

[Alex Vincent [:WeirdAl]]

brendan says this is a xpinstall bug, where xpinstall is violating ECMAScript 3rd Ed.

Comment 6

[Brendan Eich [:brendan]]

Attachment: proposed fix to xpinstall

This is what I discussed with Dan.  It seems the code wanted this sort of global Install object, with a name "Install" in its property space referring to itself, and not the two Install-class objects, one the global and the other the "Install" object in which standard classes were init'ed.

For one thing, that odd arrangement means new Function("return global") is scoped by the same object as 'var global' uses, but not as 'undeclared_global = 42' uses (the last uses the final object on the parent-linked scope chain, not the object named "Install" in that final true global).

/be

Comment 7

[Brendan Eich [:brendan]]

I need help from xpinstall users testing this.

/be

Comment 8

[Brendan Eich [:brendan]]

Attachment: proposed fix to xpinstall, v2

missed #ifdef _WINDOWS junk

Comment 9

[Alex Vincent [:WeirdAl]]

WinXP SM Trunk, I'm getting mixed results testing this.

First off, testing the URL above, I start getting some strange assertions:
###!!! ASSERTION: nsCaseConversionImp2 not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file m:/vc8/mozilla/intl/unicharutil/src/nsCaseConversionImp2.cpp, line 178
(this one three times)

###!!! ASSERTION: Using observer service off the main thread!: 'Error', file m:/vc8/mozilla/xpcom/ds/nsObserverService.cpp, line 127

###!!! ASSERTION: Using observer service off the main thread!: 'Error', file m:/vc8/mozilla/xpcom/ds/nsObserverService.cpp, line 140

Finally ending in a "Cancelled" status.  After closing the dialog by clicking on the OK button, I get:

###!!! ASSERTION: PrincipalHolder not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file m:/vc8/mozilla/js/src/xpconnect/src/xpccomponents.cpp, line 2070
(twice)

For other testcases (specifically, f_addfile.xpi), I get some of the same assertions, but also a Success instead of a Cancelled message.

From my reading of the f_adddir_empty.xpi's install.js, the cancelled message might actually be correct behavior.  I'd say that the main bug is fixed, but the assertions leave a bad taste in my mouth.

Comment 10

[Alex Vincent [:WeirdAl]]

Yeah, okay, SeaMonkey 1.0.1 reports Cancelled on the URL field as well.  So this is generally expected behavior.

Comment 11

[Alex Vincent [:WeirdAl]]

Same results on Linux.  I've verified that f_addfile.xpi correctly installs the file in the right place and by the right name.  I'd say the patch fixes the crash bug.

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 223855 [details] [diff] [review]
proposed fix to xpinstall, v2

r/sr=dveditz

Noticed a pre-existing bug: if we hit any of the error conditions in InitXPInstallObjects (new or old) then SetupInstallContext leaks the JSContext it created. Would be great to fix this too, though it would be a rare condition.

>-    glob = InitXPInstallObjects(cx, nsnull, jarFile, url, args, flags, reg, hZip);
>+    glob = InitXPInstallObjects(cx, jarFile, url, args, flags, reg, hZip);
>     if (!glob)
>         return NS_ERROR_OUT_OF_MEMORY;

Comment 13

[Brendan Eich [:brendan]]

Attachment: proposed fix to xpinstall, v3

With the fix to the unrelated leak-on-error bug dveditz pointed out.

/be

Comment 14

[Brendan Eich [:brendan]]

Fixed.

/be