In Songbird, we have some JavaScript component code that gets a string from a C++ component and uses that string as the property of a JavaScript object. This causes the string to become a JSAtom. On shutdown, the C++ component goes away, freeing the buffer used for that string. Later when the JS atom service is going away, it frees the atom which includes that same buffer, and we crash. Playing with the string in JS a bit (to turn it into a GC string?) makes the crash go away. I've traced the creation of the atom in question, and it seems that JSSTRING_IS_DEPENDENT in js_UndependString evals to false for this string, where it should probably be true.

Are you using JS_NewExternalString to create this JSString?

I'm not touching the js internals, this is all regular XPCOM JS/C++. The life of the string starts in our DatabaseEngine component when we make a nsString out of a pointer returned by sqlite: PRUnichar *p = (PRUnichar *)sqlite3_column_text16(pStmt, i); nsString strCellValue; if(p) { strCellValue = p; } This string is eventually assigned into a member variable of our sbLocalDatabaseMediaItem class. The getter the JS calls looks like: NS_IMETHODIMP sbLocalDatabaseMediaItem::GetGuid(nsAString& aGuid) { aGuid = mGuid; return NS_OK; } And the JS code looks like: this._libraryRefreshPending[aLibrary.guid] = true; If the app does this, it will crash at shutdown. To work around this, I created a js function that forces the string to become a GC string: function FIX(s) { var g = "x" + s; g = g.substr(1); return g; } And I changed all of the places where I used the string as a property: this._libraryRefreshPending[FIX(aLibrary.guid)] = true; This does not crash.

OK. Thanks for the additional detail. Yes, xpconnect can end up calling JS_NewExternalString() (see XPCStringConvert::ReadableToJSString). I *guess* this could be your probelm. js_FinalizeStringRT is the cleanup code for ordinary strings. The code that cleans up the atom table assumes atomized strings are never external. So it *might* be that external strings should never be atomized, and that's the whole problem. I don't know the best place to fix it though. You can tell whether a string is external or not in the debugger by examining *js_GetGCThingFlags(s) where s is a JSString pointer. (Note the * -- the function returns a pointer which you dereference to get the gcThingFlags, which are of type uint8). If gcThingFlags bit 0x08 is set, it's an external string.

(In reply to comment #0) > Created an attachment (id=276036) [details] > backtrace at crash > > In Songbird, we have some JavaScript component code that gets a string from a > C++ component and uses that string as the property of a JavaScript object. > This causes the string to become a JSAtom. On shutdown, the C++ component goes > away, freeing the buffer used for that string. Why does it think it owns that buffer? > Later when the JS atom service > is going away, it frees the atom which includes that same buffer, and we crash. Just from this paragraph, it sounds like you are not handling off ownership of the jschars array containing the characters to the JS engine. XPConnect has magic to do that, but if you are freeing something explicitly (an nsString's buffer?) that should be left to the platform code to manage, that could be the bug. We'd need to see your C++ component's code. /be

Sorry if i was not clear -- we do not explicitly free anything. When I said "the C++ component goes away, freeing the buffer used for that string." I may have been under the mistaken impression that buffer of an nsString member would be freed on dtor. The code involved is in our sbLocalDatabaseLibrary, which derives from sbLocalDatabaseMediaListBase, which derives from sbLocalDatabaseMediaItem. When a sbLocalDatabaseLibrary instance is created, it reads its guid from the database here: http://publicsvn.songbirdnest.com/browser/trunk/components/library/localdatabase/src/sbLocalDatabaseLibrary.cpp#L561 Then passes it to the Init method of its superclass sbLocalDatabaseMediaListBase : http://publicsvn.songbirdnest.com/browser/trunk/components/library/localdatabase/src/sbLocalDatabaseLibrary.cpp#L569 Which finally passes it to the Init method if the sbLocalDatabaseMediaItem here: http://publicsvn.songbirdnest.com/browser/trunk/components/library/localdatabase/src/sbLocalDatabaseMediaListBase.cpp#L101 And is assigned into the mGuid member here: http://publicsvn.songbirdnest.com/browser/trunk/components/library/localdatabase/src/sbLocalDatabaseMediaItem.cpp#L162 Please let me know if there is anything more I can do to help.

Where does this string enter into JS, or even XPConnect?

We get a string pointer from sqlite and assign it to an nsString here: http://publicsvn.songbirdnest.com/browser/trunk/components/dbengine/src/DatabaseEngine.cpp#L1588 That copies, right?

Yeah, that mallocs and copies. But where does this string leave your code and go into JS/XPConnect?

Here is a stack trace of a breakpoint at sbLocalDatabaseMediaItem::GetGuid that gets hit when this line executes: http://publicsvn.songbirdnest.com/browser/trunk/components/library/localdatabase/src/sbLocalDatabaseDynamicPlaylist.js#L297

(In reply to comment #4) > OK. Thanks for the additional detail. Yes, seconded. > Yes, xpconnect can end up calling JS_NewExternalString() (see > XPCStringConvert::ReadableToJSString). I *guess* this could be your probelm. > js_FinalizeStringRT is the cleanup code for ordinary strings. The code that > cleans up the atom table assumes atomized strings are never external. And that's the bug -- thanks, Jason. Coincidentally, dbaron just fixed another ancient bug to-do with intern'ed strings (JS_InternString), which caused me to look at the hardcoded finalizer call and see this bug in a flash (like dying, but less so ;-). Taking. /be

Being fixed by patch for bug 333236. /be

(In reply to comment #12) > Being fixed by patch for bug 333236. Actually I was looking for this bug to refer to it in comments since I remember seeing it, but then my search have not find it...

I keep this bug open until I update documentation about the external string finalizer usage.

I am not working on the bug right now.

who does working now?

Igor, please file a new bug for that documentation update.