nsDocument Image-Tracker Unlocking Crash [@ PL_DHashTableEnumerate | nsDocument::SetImageLockingState | UnlockEnumerator | LockEnumerator]

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

({crash})

unspecified
crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

(crash signature)

(Assignee)

Comment 1

8 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

8 years ago
blocking2.0: --- → ?

Comment 2

8 years ago
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

8 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

8 years ago
This appears to be happening only on windows.

Updated

8 years ago
blocking2.0: ? → final+
Blocks: 512260
(Assignee)

Comment 4

8 years ago
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?
(Assignee)

Comment 7

8 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

8 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).
> 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

8 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?
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

8 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
Last Resolved: 8 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.