Closed
Bug 710060
Opened 13 years ago
Closed 13 years ago
crash [@ je_free | CloseDir]
Categories
(Core :: Networking: File, defect)
Tracking
()
RESOLVED
FIXED
mozilla11
Tracking | Status | |
---|---|---|
firefox10 | --- | fixed |
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
Details
(Keywords: crash, Whiteboard: [qa-][CLOSEME:2012-03-21])
Crash Data
Attachments
(1 file, 1 obsolete file)
2.22 KB,
patch
|
bbondy
:
review+
christian
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
bp-a5bb0ffc-e274-42a6-ae0c-a0c302111207 je_free memory/jemalloc/jemalloc.c:6580 CloseDir xpcom/io/nsLocalFileWin.cpp:579 nsDirEnumerator::HasMoreElements xpcom/io/nsLocalFileWin.cpp:633 nsDeleteDir::RemoveDir netwerk/cache/nsDeleteDir.cpp:433 nsDeleteDir::RemoveDir netwerk/cache/nsDeleteDir.cpp:446 nsDeleteDir::TimerCallback netwerk/cache/nsDeleteDir.cpp:216 nsTimerImpl::Fire nsTimerEvent::Run nsThread::ProcessNextEvent nsThreadStartupEvent::`vector deleting destructor' nsThread::ThreadFunc _PR_NativeRunThread @0x7264 _getptd_noexit pr_root _callthreadstartex _threadstartex BaseThreadStart
Assignee | ||
Comment 1•13 years ago
|
||
CloseDir unconditionally deletes the nsDir arg. http://hg.mozilla.org/mozilla-central/annotate/1bd7482ad4d1/xpcom/io/nsLocalFileWin.cpp#l579 If it returns a failure code, mDir is not nulled out at the call sites. http://hg.mozilla.org/mozilla-central/annotate/1bd7482ad4d1/xpcom/io/nsLocalFileWin.cpp#l633 so I suspect the crash is a double delete on mDir.
Assignee | ||
Comment 2•13 years ago
|
||
Something like this might fix it....
Assignee | ||
Updated•13 years ago
|
Attachment #581097 -
Flags: review?(netzen)
Comment 3•13 years ago
|
||
I'm not on the XPCOM peer list but I checked with bsmedberg and he mentioned I can take the review. So I'll review it shortly.
Comment 4•13 years ago
|
||
Comment on attachment 581097 [details] [diff] [review] fix Review of attachment 581097 [details] [diff] [review]: ----------------------------------------------------------------- I think it might be cleanest to simply pass in a reference to the pointer. It would also be a patch with only a single character addition. PR_DELETE will NULL out the pointer as per: https://developer.mozilla.org/en/PR_DELETE > static nsresult > CloseDir(nsDir *&d)
Attachment #581097 -
Flags: review?(netzen)
Assignee | ||
Comment 5•13 years ago
|
||
Assignee: nobody → matspal
Attachment #581630 -
Flags: review?(netzen)
Updated•13 years ago
|
Attachment #581097 -
Attachment is obsolete: true
Comment 6•13 years ago
|
||
Comment on attachment 581630 [details] [diff] [review] fix, v2 Review of attachment 581630 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! ::: xpcom/io/nsLocalFileWin.cpp @@ +575,5 @@ > { > NS_ENSURE_ARG(d); > > BOOL isOk = FindClose(d->handle); > + PR_DELETE(d); // nulls out the slot |d| refers to (mDir) nit: please put the comment above PR_DELETE // PR_DELETE also nulls out the passed in pointer
Attachment #581630 -
Flags: review?(netzen) → review+
Assignee | ||
Comment 7•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9af6b613457
Whiteboard: [inbound]
Target Milestone: --- → mozilla10
Assignee | ||
Updated•13 years ago
|
Target Milestone: mozilla10 → mozilla11
Comment 8•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e9af6b613457
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Assignee | ||
Comment 9•13 years ago
|
||
Comment on attachment 581630 [details] [diff] [review] fix, v2 low-risk crash fix
Attachment #581630 -
Flags: approval-mozilla-beta?
Attachment #581630 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 10•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-beta/rev/9588ec86d6e6 Already in aurora as it landed on central when Fx11 was there.
status-firefox10:
--- → fixed
Comment 11•13 years ago
|
||
Is this something QA can verify? If so, what's the test case?
Whiteboard: [qa?]
Assignee | ||
Comment 13•13 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #11) > Is this something QA can verify? I wish there was a flag for "someone should come back to this bug in three months time and query crash-stats.mozilla.com for the signature(s) and reopen the bug if it still occurs in builds that have the patch". I do that sometimes... but not very systematically I'm afraid. Is that something QA can do?
Comment 14•13 years ago
|
||
We can use the closeme whiteboard tag for that. In other words, if this crash is not seen in Crashstats 3 months from now, mark bug verified.
Whiteboard: [qa-] → [qa-][CLOSEME:2012-03-21]
Comment 15•13 years ago
|
||
Sounds like a good way to verify, thanks!
You need to log in
before you can comment on or make changes to this bug.
Description
•