Closed
Bug 630193
Opened 14 years ago
Closed 14 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•14 years ago
|
blocking2.0: --- → ?
Comment 1•14 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•14 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•14 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•14 years ago
|
||
Please see also bug 630242. My tip is that a window is used after CleanUp() has been called.
Comment 5•14 years ago
|
||
#6 crash on Beta10.
Comment 6•14 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•14 years ago
|
||
If this is #6 crasher on b10, this sure should be a hardblocker.
Comment 8•14 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•14 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•14 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•14 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•14 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•14 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•14 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•14 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•14 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•14 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•14 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•14 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•14 years ago
|
||
Comment on attachment 509778 [details] [diff] [review]
v2
Looks good.
Attachment #509778 -
Flags: review?(jst) → review+
Updated•14 years ago
|
Whiteboard: [hardblocker] → [hardblocker][has patch]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
Reporter | ||
Updated•14 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•14 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•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•