Closed
Bug 588681
Opened 14 years ago
Closed 14 years ago
nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState | UnlockEnumerator | LockEnumerator]
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: bholley, Assigned: bholley)
References
Details
(Keywords: crash)
Crash Data
http://crash-stats.mozilla.com/report/index/bp-cb773b41-1820-4a55-8ece-5f96f2100818
This is a regression from bug 512260.
Assignee | ||
Comment 1•14 years ago
|
||
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.
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → ?
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
Keywords: crash
Summary: nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate ] → nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState]
Assignee | ||
Updated•14 years ago
|
Summary: nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState] → nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState | UnlockEnumerator | LockEnumerator]
Assignee | ||
Comment 3•14 years ago
|
||
This appears to be happening only on windows.
Updated•14 years ago
|
blocking2.0: ? → final+
Assignee | ||
Comment 4•14 years ago
|
||
Pushed a likely fix to this:
http://hg.mozilla.org/mozilla-central/rev/cf4d7946e2e0
Comment 5•14 years ago
|
||
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?
Comment 6•14 years ago
|
||
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?
Updated•14 years ago
|
Blocks: strongparent
Assignee | ||
Comment 7•14 years ago
|
||
(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.
Assignee | ||
Comment 8•14 years ago
|
||
> 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).
Comment 9•14 years ago
|
||
> 1) the document is unlinked from nsImageLoadingContent, causing GetOurDocument
> to return null
We should remove the requests from the tracker when that happens, no?
Assignee | ||
Comment 10•14 years ago
|
||
(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?
Comment 11•14 years ago
|
||
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.
Assignee | ||
Comment 12•14 years ago
|
||
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: 14 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Crash Signature: [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState | UnlockEnumerator | LockEnumerator]
You need to log in
before you can comment on or make changes to this bug.
Description
•