Closed
Bug 522141
Opened 15 years ago
Closed 15 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)
|
(deleted),
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
|
(deleted),
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•15 years ago
|
Attachment #406117 -
Flags: review?(brendan) → review+
|
Assignee | |
Updated•15 years ago
|
Keywords: checkin-needed
|
||
Updated•15 years ago
|
Assignee: nobody → tglek
|
|
Assignee | |
Comment 1•15 years ago
|
||
|
||
Comment 2•15 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•15 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•15 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•15 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•15 years ago
|
||
Attachment #406764 -
Attachment is obsolete: true
Attachment #406764 -
Flags: review?(brendan)
|
|
Assignee | |
Updated•15 years ago
|
Attachment #406771 -
Flags: review?(brendan)
|
|
||
Comment 7•15 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•15 years ago
|
||
|
||
Updated•15 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•15 years ago
|
|
|
Assignee | |
Comment 9•15 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 10•15 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•13 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
•