Closed
Bug 630193
Opened 13 years ago
Closed 13 years ago
Crash [@ PL_DHashTableOperate | nsTHashtable<nsUniCharEntry>::PutEntry(unsigned short const*)][@ PL_DHashTableOperate | nsTHashtable<nsBaseHashtableET<nsUint32HashKey, nsAutoPtr<`anonymous namespace''::DatabaseInfoHash> > >::PutEntry(unsigned int const&)]
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b12
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: scoobidiver, Assigned: mayhemer)
References
Details
(Keywords: crash, regression, topcrash, Whiteboard: [hardblocker][has patch])
Crash Data
Attachments
(1 file, 1 obsolete file)
3.20 KB,
patch
|
jst
:
review+
khuey
:
feedback+
|
Details | Diff | Splinter Review |
It is a new crash signature in 4.0b10 (does not exist in 4.0b9). It is #13 top crasher in 4.0b10 over the last week. Signature PL_DHashTableOperate | nsTHashtable<nsUniCharEntry>::PutEntry(unsigned short const*) UUID 2b8d85be-96a6-4052-947e-6cc022110131 Time 2011-01-31 03:48:51.438510 Uptime 108 Last Crash 3452235 seconds (5.7 weeks) before submission Install Age 55369 seconds (15.4 hours) since version was first installed. Product Firefox Version 4.0b10 Build ID 20110121161358 Branch 2.0 OS Windows NT OS Version 6.1.7600 CPU x86 CPU Info GenuineIntel family 6 model 15 stepping 13 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0x9 App Notes AdapterVendorID: 8086, AdapterDeviceID: 2a02, AdapterDriverVersion: 8.15.10.1930 Frame Module Signature [Expand] Source 0 xul.dll PL_DHashTableOperate obj-firefox/xpcom/build/pldhash.c:615 1 xul.dll nsTHashtable<nsUniCharEntry>::PutEntry obj-firefox/dist/include/nsTHashtable.h:188 2 xul.dll nsBaseHashtable<nsStringHashKey,int,int>::Put obj-firefox/dist/include/nsBaseHashtable.h:163 3 xul.dll nsGlobalWindow::Observe 4 @0x15f 5 mozcrt19.dll arena_dalloc_small obj-firefox/memory/jemalloc/crtsrc/jemalloc.c:4153 6 xul.dll nsTArray_base<nsTArrayDefaultAllocator>::ShiftData obj-firefox/dist/include/nsTArray-inl.h:169 7 xul.dll nsObserverList::FillObserverArray xpcom/ds/nsObserverList.cpp:119 8 xul.dll PL_DHashTableOperate obj-firefox/xpcom/build/pldhash.c:625 9 xul.dll xul.dll@0xbbda57 10 @0x2e0076 11 xul.dll nsAString_internal::Assign xpcom/string/src/nsTSubstring.cpp:391 12 xul.dll nsAString_internal::Finalize xpcom/string/src/nsTSubstring.cpp:188 13 xul.dll DOMStorageImpl::SetValue dom/src/storage/nsDOMStorage.cpp:1235 14 xul.dll nsDOMStorage::SetItem dom/src/storage/nsDOMStorage.cpp:1633 15 xul.dll nsStorageSH::SetProperty dom/base/nsDOMClassInfo.cpp:10324 16 xul.dll XPC_WN_Helper_SetProperty js/src/xpconnect/src/xpcwrappednativejsops.cpp:987 17 mozjs.dll js::Shape::set js/src/jsscopeinlines.h:275 18 mozjs.dll js_NativeSet js/src/jsobj.cpp:5199 19 mozjs.dll js_SetPropertyHelper js/src/jsobj.cpp:5674 20 mozjs.dll js_SetProperty js/src/jsobj.cpp:5685 21 mozjs.dll js::mjit::stubs::SetElem<0> js/src/methodjit/StubCalls.cpp:591 More reports at: https://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&signature=PL_DHashTableOperate%20|%20nsTHashtable%3CnsUniCharEntry%3E%3A%3APutEntry%28unsigned%20short%20const*%29
Reporter | ||
Updated•13 years ago
|
blocking2.0: --- → ?
Comment 1•13 years ago
|
||
That is a very strange stack. The bit up to DOMStorageImpl::SetValue makes sense, but I don't really understand where the rest of it comes from.
Comment 2•13 years ago
|
||
Ok, the hashtable frames could be coming from nsGlobalWindow::Observe, ie. > if (!mPendingStorageEventsObsolete) { > mPendingStorageEventsObsolete = new nsDataHashtable<nsStringHashKey, PRBool>; > NS_ENSURE_TRUE(mPendingStorageEventsObsolete, NS_ERROR_OUT_OF_MEMORY); > > rv = mPendingStorageEventsObsolete->Init(); > NS_ENSURE_SUCCESS(rv, rv); > } > > mPendingStorageEventsObsolete->Put(domain, PR_TRUE); but it's still weird how the frames between the DOMStorage value setting and the observer notification are screwed up/nonexistent in some reports.
Comment 3•13 years ago
|
||
Interestingly enough, every single report I've looked at so far looks to be from some European version of Windows.
![]() |
Assignee | |
Comment 4•13 years ago
|
||
Please see also bug 630242. My tip is that a window is used after CleanUp() has been called.
Comment 5•13 years ago
|
||
#6 crash on Beta10.
Comment 6•13 years ago
|
||
The stack above nsGlobalWindow::Observe is probably completely bogus. Honza seems to have a clue what's going on, so he gets the bug ;-)
Comment 7•13 years ago
|
||
If this is #6 crasher on b10, this sure should be a hardblocker.
Comment 8•13 years ago
|
||
I think bsmedberg meant to assign this to Honza, and having him take a closer look makes sense to me.
Whiteboard: [hardblocker]
![]() |
Assignee | |
Comment 9•13 years ago
|
||
My theory is that it is related to bug 630242. We are sending DOM storage events to an nsGlobalWindow instance that is both: - CleanUp()'ed - Freeze()'ed Going to check if I really am able to reproduce the first condition with test case from bug 630242. If so, then fixing it will probably fix this bug as well.
Assignee: nobody → honzab.moz
![]() |
Assignee | |
Comment 10•13 years ago
|
||
This could be a dupe of some nsGlobalWindow leak. We remove observers in the destructor. Could this be that fix for bug 622361 is not sufficient?
![]() |
Assignee | |
Updated•13 years ago
|
Status: NEW → ASSIGNED
Well, the fix for Bug 622361 doesn't really mess with old style storage events. (I don't think moving the delete call should have changed anything.)
![]() |
Assignee | |
Comment 12•13 years ago
|
||
OK, I think I can say it is what I describe in comment 9, with additional conditions, enumerating them rather all again: - nsGlobalWindow must receive an event from globalStorage change while it is frozen (to create mPendingStorageEventsObsolete hash table) - it's Freeze()'ed and it's CleanUp()'ed (mPendingStorageEventsObsolete deleted but not nullified) ; doesn't matter in what order - receive globalStorage change event again I just checked I was able to receive globalStorage event in a window that has been CleanUp()'ed. To have a simple fix for this bug, I'd return early from ::Observe() method if mCleanedUp flag is set. Also nullifying mPendingStorageEventsObsolete would be good. Someone more experienced with lifetime of nsGlobalWindow instance should confirm it is OK. Kyle, I don't think anymore it could be related to your changes, this bug is there IMO forever, just raised because of some other change that makes the four conditions appear more often.
![]() |
Assignee | |
Comment 13•13 years ago
|
||
Patch according to comment 12. Who's gonna review it?
Attachment #509151 -
Flags: review?
Should probably switch mPendingStorageEventsObsolete to be an nsAutoPtr. Maybe nsGlobalWindow should just stop observing these events when it is cleaned up. Bailing on *all* notifications when the window has been cleaned up seems risky both now and in the future.
Comment 15•13 years ago
|
||
Comment on attachment 509151 [details] [diff] [review] v1 I think this would fix this, and I'd be ok with this change for now, but I think we might as well do what Kyle suggests and make this more future proof.
Attachment #509151 -
Flags: review? → review-
![]() |
Assignee | |
Comment 16•13 years ago
|
||
I agree. I am just testing the patch as suggested by Kyle, and have following issues: I'm getting a different scenario now. I can still see an nsGlobalWindow instance, that has no outer window, is NOT CleanUp()'ed - the flag is not set - and in general seems 'empty', i.e. all members of nsPIDOMWindow are null. However, in the log (DOMLeak:5) I can see that the window has been created and assigned an outer window in the constructor, and so far not destroyed. As I'm looking to mxr, I cannot find a place where mOuterWindow should be released (nullified). Could the cycle collector free it up? Though, there are other inner windows referring the same outer window that are still alive. Could it be some saved state reversal?
![]() |
Assignee | |
Comment 17•13 years ago
|
||
(In reply to comment #16) > and in general seems 'empty', i.e. all members of nsPIDOMWindow are null. Except: - mIsInnerWindow - mWindowID - mHasNotifiedGlobalCreated
Comment 18•13 years ago
|
||
When the cycle collector unlinks an inner window mOuterWindow is nulled out (at http://hg.mozilla.org/mozilla-central/annotate/bc9d7938cdc7/dom/base/nsGlobalWindow.cpp#l1390).
![]() |
Assignee | |
Comment 19•13 years ago
|
||
Yes, that's it, just adding a simple log ("DOMWINDOW %p UNLINK", static_cast<nsGlobalWindow*>(tmp)) to NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGlobalWindow) section shows it is the case. I'll move comments 16 to 19 to bug 630242 where those belong. It seems unrelated to this bug now, just confused me.
![]() |
Assignee | |
Comment 20•13 years ago
|
||
- mPendingStorageEventsObsolete turned to nsAutoPtr - observer removal moved to CleanUp() method
Attachment #509151 -
Attachment is obsolete: true
Attachment #509778 -
Flags: review?(khuey)
Comment on attachment 509778 [details] [diff] [review] v2 I think this is ok, but it really should be reviewed by jst or another dom peer.
Attachment #509778 -
Flags: review?(khuey)
Attachment #509778 -
Flags: review?(jst)
Attachment #509778 -
Flags: feedback+
Comment 22•13 years ago
|
||
Comment on attachment 509778 [details] [diff] [review] v2 Looks good.
Attachment #509778 -
Flags: review?(jst) → review+
Updated•13 years ago
|
Whiteboard: [hardblocker] → [hardblocker][has patch]
http://hg.mozilla.org/mozilla-central/rev/d907bea5f40f
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
Reporter | ||
Updated•13 years ago
|
Summary: Crash [@ PL_DHashTableOperate | nsTHashtable<nsUniCharEntry>::PutEntry(unsigned short const*) ] → nsAutoPtr<`anonymous namespace''::DatabaseInfoHash> > >::PutEntry(unsigned int const&)] Crash [@ PL_DHashTableOperate | nsTHashtable<nsUniCharEntry>::PutEntry(unsigned short const*)][@ PL_DHashTableOperate | nsTHashtable<nsBaseHashtableET<nsUint32HashKey
Updated•12 years ago
|
Crash Signature: [@ PL_DHashTableOperate | nsTHashtable<nsUniCharEntry>::PutEntry(unsigned short const*)]
[@ PL_DHashTableOperate | nsTHashtable<nsBaseHashtableET<nsUint32HashKey, nsAutoPtr<`anonymous namespace''::DatabaseInfoHash> > >::PutEntry(unsigned int const&)]
Updated•4 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•