Closed
Bug 522141
Opened 14 years ago
Closed 14 years ago
FastLoadFileReader fails to close if an error occurs during opening [@nsCOMPtr_base::assign_with_AddRef(nsISupports*) ]
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
People
(Reporter: taras.mozilla, Assigned: taras.mozilla)
References
Details
(Keywords: topcrash, Whiteboard: [#3 trunk (3.7a1) topcrash caused by first patch and fixed by third])
Crash Data
Attachments
(2 files, 1 obsolete file)
1.24 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
472 bytes,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
Ben Turner debugged this one on windows where failing to close a mmapped file prevents a new one from being created.
Attachment #406117 -
Flags: review?(brendan)
Updated•14 years ago
|
Attachment #406117 -
Flags: review?(brendan) → review+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Updated•14 years ago
|
Assignee: nobody → tglek
Assignee | ||
Comment 1•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/a7a756bc341b
Comment 2•14 years ago
|
||
There's a problem here in that mNumSharpObjects is not initialised if an error occurs during opening, so we now fail to close harder, i.e. we crash ;-) I don't see an easy fix for the case where ReadFooterPrefix reads in a non-zero mNumSharpObjects but fails to read the object map.
Assignee | ||
Comment 3•14 years ago
|
||
(In reply to comment #2) > There's a problem here in that mNumSharpObjects is not initialised if an error > occurs during opening, so we now fail to close harder, i.e. we crash ;-) > > I don't see an easy fix for the case where ReadFooterPrefix reads in a non-zero > mNumSharpObjects but fails to read the object map. Good point, hope this isn't an actual crash you ran into. i'll look addressing that soon
Assignee | ||
Comment 4•14 years ago
|
||
i doubt that this is a likely crash, but doesn't hurt to guard against a null deref.
Attachment #406764 -
Flags: review?(brendan)
Comment 5•14 years ago
|
||
Comment on attachment 406764 [details] [diff] [review] safety fix >diff --git a/xpcom/io/nsFastLoadFile.cpp b/xpcom/io/nsFastLoadFile.cpp >--- a/xpcom/io/nsFastLoadFile.cpp >+++ b/xpcom/io/nsFastLoadFile.cpp >@@ -924,7 +924,7 @@ nsFastLoadFileReader::Close() > PR_Close(mFd); > mFd = nsnull; > } >- for (PRUint32 i = 0, n = mFooter.mNumSharpObjects; i < n; i++) { >+ for (PRUint32 i = 0, n = mFooter.mNumSharpObjects; mFooter.mObjectMap && i < n; i++) { Use an if please -- should Close just return early before the for loop if mObjectMap is null here? > nsObjectMapEntry* entry = &mFooter.mObjectMap[i]; > entry->mReadObject = nsnull; > } >diff --git a/xpcom/proxy/src/nsProxyObjectManager.cpp b/xpcom/proxy/src/nsProxyObjectManager.cpp >--- a/xpcom/proxy/src/nsProxyObjectManager.cpp >+++ b/xpcom/proxy/src/nsProxyObjectManager.cpp >@@ -333,7 +333,7 @@ NS_GetProxyForObject(nsIEventTarget *tar > do_GetService(proxyObjMgrCID, &rv); > if (NS_FAILED(rv)) > return rv; >- >+ //getClassObjectByContractID What's this? Trailing whitespace and no space after "//" -- looks like a glitch. /be > // and try to get the proxy object > // > return proxyObjMgr->GetProxyForObject(target, aIID, aObj,
Assignee | ||
Comment 6•14 years ago
|
||
Attachment #406764 -
Attachment is obsolete: true
Attachment #406764 -
Flags: review?(brendan)
Assignee | ||
Updated•14 years ago
|
Attachment #406771 -
Flags: review?(brendan)
Comment 7•14 years ago
|
||
Comment on attachment 406771 [details] [diff] [review] safety fix r=me with a brief comment on what causes that condition. /be
Attachment #406771 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 8•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/1c4fed24fc4c
Updated•14 years ago
|
Keywords: topcrash
Summary: FastLoadFileReader fails to close if an error occurs during opening → FastLoadFileReader fails to close if an error occurs during opening [@nsCOMPtr_base::assign_with_AddRef(nsISupports*) ]
Whiteboard: [#3 trunk (3.7a1) topcrash caused by first patch and fixed by third]
Updated•13 years ago
|
Assignee | ||
Comment 11•13 years ago
|
||
I dont see a comment 9 in this bug. Goes from 8 to 10(weird). This fix was for trunk changes that haven't landed on 1.9.2.
Comment 13•13 years ago
|
||
This is a fixed fastload bug, and should not be confused with an unfixed layout bug just because both of them crash in the same way.
Updated•12 years ago
|
Crash Signature: [@nsCOMPtr_base::assign_with_AddRef(nsISupports*) ]
You need to log in
before you can comment on or make changes to this bug.
Description
•