Closed Bug 588681 Opened 11 years ago Closed 11 years ago
Document Image-Tracker Unlocking Crash [@ PL _DHash Table Enumerate | ns Document::Set Image Locking State | Unlock Enumerator | Lock Enumerator]
http://crash-stats.mozilla.com/report/index/bp-cb773b41-1820-4a55-8ece-5f96f2100818 This is a regression from bug 512260.
Just as a note - if this becomes serious enough for someone to intervene on trunk before I have a chance to figure it out, the best solution is not a backout, but to just comment out the call to EnumerateRead in nsDocument::SetImageLockingState.
Signature PL_DHashTableEnumerate UUID cb773b41-1820-4a55-8ece-5f96f2100818 Time 2010-08-18 20:07:16.870806 Uptime 20567 Last Crash 1930609 seconds (3.2 weeks) before submission Install Age 20847 seconds (5.8 hours) since version was first installed. Product Firefox Version 4.0b5pre Build ID 20100818112603 Branch 2.0 OS Windows NT OS Version 6.1.7600 CPU x86 CPU Info GenuineIntel family 6 model 15 stepping 6 Crash Reason EXCEPTION_ACCESS_VIOLATION Crash Address 0x1 User Comments Not sure :/ Processor Notes EMCheckCompatibility False Related Bugs Crashing Thread Frame Module Signature [Expand] Source 0 @0x1 1 xul.dll PL_DHashTableEnumerate obj-firefox/xpcom/build/pldhash.c:754 2 xul.dll nsDocument::SetImageLockingState content/base/src/nsDocument.cpp:8057 3 xul.dll nsDocument::~nsDocument content/base/src/nsDocument.cpp:1583 4 xul.dll nsHTMLDocument::~nsHTMLDocument 5 xul.dll nsHTMLDocument::~nsHTMLDocument 6 xul.dll nsHTMLDocument::`scalar deleting destructor' 7 xul.dll nsNodeUtils::LastRelease content/base/src/nsNodeUtils.cpp:294 8 xul.dll nsXMLDocument::Release content/html/document/src/nsHTMLDocument.cpp:276 9 xul.dll XPCWrappedNative::FlatJSObjectFinalized js/src/xpconnect/src/xpcwrappednative.cpp:1401 10 xul.dll XPC_WN_NoHelper_Finalize js/src/xpconnect/src/xpcwrappednativejsops.cpp:666 11 xul.dll FinalizeArenaList<JSObject,&FinalizeObject> js/src/jsgc.cpp:2796 12 xul.dll GC js/src/jsgc.cpp:3122 13 xul.dll GCUntilDone js/src/jsgc.cpp:3442 14 xul.dll js_GC js/src/jsgc.cpp:3496 15 xul.dll JS_GC js/src/jsapi.cpp:2453 16 xul.dll nsXPConnect::Collect js/src/xpconnect/src/nsXPConnect.cpp:402 17 xul.dll nsXPConnect::GarbageCollect js/src/xpconnect/src/nsXPConnect.cpp:410 18 xul.dll nsJSContext::CC dom/base/nsJSEnvironment.cpp:3633
Severity: normal → critical
Summary: nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate ] → nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState]
Summary: nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState] → nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState | UnlockEnumerator | LockEnumerator]
This appears to be happening only on windows.
Pushed a likely fix to this: http://hg.mozilla.org/mozilla-central/rev/cf4d7946e2e0
After some changes to document tear down, I get this pretty consistently on shutdown (linux). Adding tmp->SetImageLockingState(PR_FALSE); to documents unlink helps, but that is just a hack. I don't understand how mImageTracker works. It has nsPtrHashKey<imgIRequest> as the key, but what keeps imgIRequest alive long enough?
So, note, HTMLImageElements can stay alive longer than their ownerDocument. Is it possible that there is some assumption that elements wouldn't stay alive longer than their document?
(In reply to comment #6) > So, note, HTMLImageElements can stay alive longer than their ownerDocument. > Is it possible that there is some assumption that elements wouldn't stay > alive longer than their document? (discussed on IRC, posting here for posterity) It shouldn't be a problem for images to outlive their documents, because the document image tracker clears itself during destruction. See the comment here: http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsImageLoadingContent.cpp#996 However, a crash would occur if the following three things happened in order: 1) the document is unlinked from nsImageLoadingContent, causing GetOurDocument to return null 2) nsImageLoadingContent::Destroy() is called, calling its imgIRequests to be released 3) somebody calls nsDocument::SetImageLockingState(true|false), causing method calls to the stale imgIRequests in the tracker. An easy fix would be to addref/release the images when they go in and out of the document tracker. However, it'd be good to know first what's actually going on.
> 1) the document is unlinked from nsImageLoadingContent, causing GetOurDocument > to return null And by this I just mean that the pointer is null-ed out (apparently unlink refers to cycle collector stuff, which may or may not be relevant here).
> 1) the document is unlinked from nsImageLoadingContent, causing GetOurDocument > to return null We should remove the requests from the tracker when that happens, no?
(In reply to comment #9) > We should remove the requests from the tracker when that happens, no? If there's an easy way to do it, that would be a good idea. Is there?
Mm... nsGenericElement::Unlink does not, at the moment, null out the document stuff. So the only way for the owner document to become null, as far as I can tell, is if the owner document has actually had ~nsDocument called on it.
Ok, so it looks like all the crash reports for this stop after the push mentioned in comment 4. So I'm going to resolve this fixed. Smaug, if you can reproduce this on a clean trunk, let me know. Otherwise, if you're triggering it by changing document teardown, we should probably talk about that in a separate bug.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Crash Signature: [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState | UnlockEnumerator | LockEnumerator]
You need to log in before you can comment on or make changes to this bug.