FileHandle: Fix the assertion in FileHelper::~FileHelper

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: janv, Assigned: janv)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

fix
856 bytes, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Assertion:
###!!! ASSERTION: Wrong thread!: 'NS_IsMainThread()', file e:/builds/moz2_slave/try-w32-dbg/build/dom/file/FileHelper.cpp, line 39


Simple description:
nsAsyncStreamCopier (which we use for reading and writing) can become the last owner of a FileHelper


More detailed description:
nsAsyncStreamCopier::Complete() which is running on a background thread calls
observer->OnStopRequest()

The observer is a proxy, so OnStopRequest() is called on the main thread.

However, FileHelper::OnStopRequest() calls FileHelper::Finish() which removes
other references to the FileHelper, and also a strong ref to the stream copier.
This can happen before nsAsyncStreamCopier::Complete() finishes, so nsAsyncStreamCopier becomes the last owner of a FileHelper


I suggest to just change the assertion in FileHelper::~FileHelper to:
NS_ASSERTION(!mFileStorage && !mLockedFile && !mFileRequest && !mListener &&
             !mRequest, "Should have cleared this!");
(Assignee)

Comment 1

5 years ago
Actually, it can be also released through a bit different code path:

xul!mozilla::dom::file::FileOutputStreamWrapper::Release+0x000000000000000D (c:
sources\mozilla\mozilla-central\dom\file\filestreamwrappers.cpp, line 238)
xul!nsCOMPtr<nsIOutputStream>::~nsCOMPtr<nsIOutputStream>+0x000000000000003B (c
\sources\mozilla\obj-ff-dbg\dist\include\nscomptr.h, line 488)
xul!nsAsyncStreamCopier::~nsAsyncStreamCopier+0x0000000000000074 (c:\sources\mo
illa\mozilla-central\netwerk\base\src\nsasyncstreamcopier.cpp, line 42)
xul!nsAsyncStreamCopier::`scalar deleting destructor'+0x000000000000000F
xul!nsAsyncStreamCopier::Release+0x000000000000008F (c:\sources\mozilla\mozilla
central\netwerk\base\src\nsasyncstreamcopier.cpp, line 95)
xul!nsAsyncStreamCopier::OnAsyncCopyComplete+0x0000000000000024 (c:\sources\moz
lla\mozilla-central\netwerk\base\src\nsasyncstreamcopier.cpp, line 87)
xul!nsAStreamCopier::Process+0x0000000000000423 (c:\sources\mozilla\mozilla-cen
ral\xpcom\io\nsstreamutils.cpp, line 364)
xul!nsAStreamCopier::Run+0x0000000000000011 (c:\sources\mozilla\mozilla-central
xpcom\io\nsstreamutils.cpp, line 405)
xul!nsThreadPool::Run+0x00000000000002A0 (c:\sources\mozilla\mozilla-central\xp
om\threads\nsthreadpool.cpp, line 187)
xul!nsThread::ProcessNextEvent+0x000000000000034B (c:\sources\mozilla\mozilla-c
ntral\xpcom\threads\nsthread.cpp, line 624)
xul!NS_ProcessNextEvent_P+0x0000000000000054 (c:\sources\mozilla\obj-ff-dbg\xpc
m\build\nsthreadutils.cpp, line 213)
xul!nsThread::ThreadFunc+0x00000000000000D4 (c:\sources\mozilla\mozilla-central
xpcom\threads\nsthread.cpp, line 257)

so, nsAsyncStreamCopier still becomes the last indirect owner of FileHelper, but through a ref to stream, FileOutputStreamWrapper which holds FileHelper
Assignee: nobody → Jan.Varga
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
Created attachment 631647 [details] [diff] [review]
fix
Attachment #631647 - Flags: review?(bent.mozilla)
Attachment #631647 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 3

5 years ago
http://hg.mozilla.org/mozilla-central/rev/6a2100ce978f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.