xpcshell core dump when shutdown

RESOLVED FIXED in mozilla1.9beta2

Status

()

Core
JavaScript Engine
P2
normal
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Solar Flare, Assigned: Solar Flare)

Tracking

Trunk
mozilla1.9beta2
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

11 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) Gecko/2007110515 Minefield/3.0a9pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) 

back trace:

Core was generated by `./xpcshell'.
Program terminated with signal 5, Trace/breakpoint trap.
#0  JS_Assert (s=0xb7ebe097 "!rt->deflatedStringCacheLock", 
    file=0xb7ebdb80 "/home/solar/mozilla/js/src/jsstr.c", ln=2349)
    at /home/solar/mozilla/js/src/jsutil.c:63
63          abort();
(gdb) bt
#0  JS_Assert (s=0xb7ebe097 "!rt->deflatedStringCacheLock", 
    file=0xb7ebdb80 "/home/solar/mozilla/js/src/jsstr.c", ln=2349)
    at /home/solar/mozilla/js/src/jsutil.c:63
#1  0xb7e8b631 in js_InitRuntimeStringState (cx=0x8094710)
    at /home/solar/mozilla/js/src/jsstr.c:2349
#2  0xb7ddd541 in js_NewContext (rt=0x808eec0, stackChunkSize=8192)
    at /home/solar/mozilla/js/src/jscntxt.c:297
#3  0xb7dc8024 in JS_NewContext (rt=0x808eec0, stackChunkSize=8192)
    at /home/solar/mozilla/js/src/jsapi.c:990
#4  0xb789b82a in XPCJSContextStack::GetSafeJSContext (this=0x808eb88, 
    aSafeJSContext=0xbfb87110)
    at /home/solar/mozilla/js/src/xpconnect/src/xpcthreadcontext.cpp:206
#5  0xb786b407 in XPCCallContext (this=0xbfb870f4, 
    callerLanguage=XPCContext::LANG_NATIVE, cx=0x0, obj=0x0, funobj=0x0, 
    name=0, argc=4294967295, argv=0x0, rval=0x0)
    at /home/solar/mozilla/js/src/xpconnect/src/xpccallcontext.cpp:93
#6  0xb78659af in nsXPConnect::Collect (this=0x8095160)
    at /home/solar/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:490
#7  0xb7d614b7 in nsCycleCollector::Collect (this=0x806e8b8, aTryCollections=5)
    at /home/solar/mozilla/xpcom/base/nsCycleCollector.cpp:2094
#8  0xb7d61589 in nsCycleCollector::Shutdown (this=0x806e8b8)
    at /home/solar/mozilla/xpcom/base/nsCycleCollector.cpp:2249
#9  0xb7d615c0 in nsCycleCollector_shutdown ()
---Type <return> to continue, or q <return> to quit---
    at /home/solar/mozilla/xpcom/base/nsCycleCollector.cpp:2667
#10 0xb7cd71ea in NS_ShutdownXPCOM_P (servMgr=0x0)
    at /home/solar/mozilla/xpcom/build/nsXPComInit.cpp:785
#11 0xb7dab7c7 in NS_ShutdownXPCOM (svcMgr=0x0)
    at /home/solar/mozilla/xpcom/stub/nsXPComStub.cpp:175
#12 0x0804c234 in main (argc=0, argv=0xbfb873c8, envp=0xbfb873cc)
    at /home/solar/mozilla/js/src/xpconnect/shell/xpcshell.cpp:1459
Current language:  auto; currently c
(gdb)

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Version: unspecified → Trunk
(Assignee)

Comment 1

11 years ago
Created attachment 287498 [details] [diff] [review]
change the initialization of string hash

Per igor's suggestion, the string hash(and its lock) should be initialized in JS_NewRuntime rather than JS_NewContext. (Comment #51 in bug 401687)
I had updated the patch.
Attachment #287498 - Flags: review+
Attachment #287498 - Attachment description: change the initaliaztion of string hash → change the initialization of string hash
Attachment #287498 - Flags: review+ → review?(igor)
Assignee: general → solar
Status: UNCONFIRMED → NEW
Ever confirmed: true
How common is it to hit this? This sounds like something we really don't want to ship beta with.
Flags: blocking1.9?
Target Milestone: --- → mozilla1.9 M9

Comment 3

11 years ago
Comment on attachment 287498 [details] [diff] [review]
change the initialization of string hash

>+++ js/src/jsstr.c	5 Nov 2007 10:19:06 -0000
> JSBool
> js_InitRuntimeStringState(JSContext *cx)
> {
>+    rt->deflatedStringCache->keyHash=js_hash_string_pointer;
>     rt->emptyString = ATOM_TO_STRING(rt->atomState.emptyAtom);
>     return JS_TRUE;
> }
>-void
>-js_FinishDeflatedStringCache(JSRuntime *rt)
>-{
>-    if (rt->deflatedStringCache) {
>-        JS_HashTableDestroy(rt->deflatedStringCache);
>-        rt->deflatedStringCache = NULL;
>-    }
>-#ifdef JS_THREADSAFE
>-    if (rt->deflatedStringCacheLock) {
>-        JS_DESTROY_LOCK(rt->deflatedStringCacheLock);
>-        rt->deflatedStringCacheLock = NULL;
>-    }
>-#endif
>-}

To minimize the patch and avoid the ugliness of rt->deflatedStringCache->keyHash=js_hash_string_pointer I suggest to keep js_FinishDeflatedStringCache and just add a parallel 

JSBool 
js_InitDeflatedStringCache(JSRuntime *rt);

to be called from the runtime initialization code. 

r+ with these changes.
(Assignee)

Comment 4

11 years ago
(In reply to comment #2)
> How common is it to hit this? This sounds like something we really don't want
> to ship beta with.
> 

xpcshell will crash when components/compreg.dat exist.
Status: NEW → ASSIGNED
Comment on attachment 287498 [details] [diff] [review]
change the initialization of string hash

Yeah, and space on each side of = and other operators.

/be
Attachment #287498 - Flags: review?(igor) → review-
Actually, does this only affect xpcshell? If so I don't think this is a beta blocker.
Not blocking M9, I'll leave it to component owners to decide overall blocking status.
Target Milestone: mozilla1.9 M9 → ---
(Assignee)

Comment 8

11 years ago
Created attachment 287651 [details] [diff] [review]
add js_InitDeflatedStringCache

I had updated the patch. This time, I add a new js_InitDeflatedStringCache function in jsstr.c. It will be called when create a new JSRuntime. This correspond with the js_FinishDeflatedStringCache function which will be called when destroy JSRuntime.

This bug currently affects xpcshell only. But it's a potential issue of firefox I think.
Attachment #287498 - Attachment is obsolete: true
Attachment #287651 - Flags: review?(igor)

Comment 9

11 years ago
Though the patch needs review, it looks good or close, and this bug very likely -does- affect more than just xpcshell.  Not sure why Fx is getting lucky right now.
Priority: -- → P2
Flags: blocking1.9? → blocking1.9+

Comment 10

11 years ago
Comment on attachment 287651 [details] [diff] [review]
add js_InitDeflatedStringCache

>Index: js/src/jsstr.c
> void
> js_PurgeDeflatedStringCache(JSRuntime *rt, JSString *str)
> {
>     JSHashNumber hash;
>     JSHashEntry *he, **hep;
> 
>     if (!rt->deflatedStringCache)
>         return;

This is useless check as with the patch js_PurgeDeflatedStringCache should never be called with not initialized cache. r+ with that fixed.

Comment 11

11 years ago
instead, JS_ASSERT(rt->deflatedStringCache) ?

Comment 12

11 years ago
(In reply to comment #11)
> instead, JS_ASSERT(rt->deflatedStringCache) ?

The assert is not necessary as the CPU will do the check when dereferencing the null pointer. I.e. SpiderMonkey's style is not to assert that a pointer is not null if the pointer is dereferenced on all code paths. 
(Assignee)

Comment 13

11 years ago
Created attachment 288805 [details] [diff] [review]
Remove the useless check

Remove the useless check in js_PurgeDeflatedStringCache.
Attachment #287651 - Attachment is obsolete: true
Attachment #288805 - Flags: review?(igor)
Attachment #287651 - Flags: review?(igor)

Updated

11 years ago
Attachment #288805 - Flags: review?(igor) → review+
Thanks for the patch!

Checking in js/src/jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.370; previous revision: 3.369
done
Checking in js/src/jsstr.c;
/cvsroot/mozilla/js/src/jsstr.c,v  <--  jsstr.c
new revision: 3.175; previous revision: 3.174
done
Checking in js/src/jsstr.h;
/cvsroot/mozilla/js/src/jsstr.h,v  <--  jsstr.h
new revision: 3.54; previous revision: 3.53
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M10
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.