Closed Bug 1287747 Opened 3 years ago Closed 3 years ago

Intermittent dom/filesystem/compat/tests/test_formSubmission.html,test_worker_basic.html | application crashed [@ mozilla::dom::ContentParent::Release] or [@ mozilla::dom::Blob::Release]

Categories

(Core :: DOM: Content Processes, defect, P3)

Product:
Core
Component:
DOM: Content Processes
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla51
Tracking Flags:
Tracking Status
firefox49 --- unaffected
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: baku)

References

Details

(Keywords: crash, intermittent-failure)

Attachments

(4 files, 2 obsolete files)

getFiles_crash.patch
2.86 KB, patch
smaug
: review+
Details | Diff | Splinter Review
getFiles_crash2.patch
2.27 KB, patch
smaug
: review+
Details | Diff | Splinter Review
getFiles_crash3.patch
2.71 KB, patch
smaug
: review+
Details | Diff | Splinter Review
roll-up patch for beta uplift
5.09 KB, patch
baku
: feedback+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
Component: DOM → DOM: Content Processes
Andrea, looks like you wrote the crashing code in question. Can you please take a look when you get a chance?
Blocks: 1186932
status-firefox49: --- → unaffected
status-firefox50: --- → affected
status-firefox51: --- → affected
Flags: needinfo?(amarchesini)
Keywords: crash
There is 
libxul.so!mozilla::ipc::MessagePumpForNonMainThreads::Run [MessagePump.cpp:2c5773f47093 : 338 + 0x9]  on stack.
Are we releasing ContentParent in non-main-thread. That would be bad.
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attachment #8789990 - Flags: review?(bugs)
On which all branches do we need this?
Flags: needinfo?(amarchesini)
Attachment #8789990 - Flags: review?(bugs) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e44c3152e4bc
GetFilesHelper must release callbacks and promises on the main-thread only, r=smaug
Comment on attachment 8789990 [details] [diff] [review]
getFiles_crash.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1186932
[User impact if declined]: Possibly a crash can occur.
[Describe test coverage new/current, TreeHerder]: None. Almost impossible to reproduce programmatically. 
[Risks and why]: no big ones: if GetFilesHelper is deleted on a different thread, it dispatches a runnable in order to release the internal promises and callbacks into the main-thread.
[String/UUID change made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8789990 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e44c3152e4bc
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Doesn't actually seem to be fixed, still seeing it crash, e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=35694899&repo=mozilla-inbound
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8789990 [details] [diff] [review]
getFiles_crash.patch

Based on the last comment, this fix didn't really work. Given that, I don't see a reason to uplift to Aurora50.
Attachment #8789990 - Flags: approval-mozilla-aurora?
GetFilesHelperParent is a GetFilesHelper and it can be released in any thread.
Attachment #8790953 - Flags: review?(bugs)
Attachment #8790953 - Flags: review?(bugs) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce4e1e8aaa0e
GetFilesHelperParent must release ContentParent on the main-thread, r=smaug
https://hg.mozilla.org/mozilla-central/rev/ce4e1e8aaa0e
Status: REOPENED → RESOLVED
Closed: 3 years ago → 3 years ago
Resolution: --- → FIXED
Looks like it's failing with a different signature now.
https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-inbound&job_id=35880441

12:22:26  WARNING -  PROCESS-CRASH | dom/filesystem/compat/tests/test_formSubmission.html | application crashed [@ mozilla::dom::Blob::Release]
12:22:26     INFO -  Crash dump filename: /tmp/tmp_UfQLE.mozrunner/minidumps/0b5415aa-ebef-8a24-085ec404-559c7564.dmp
12:22:26     INFO -  Operating system: Linux
12:22:26     INFO -                    0.0.0 Linux 3.2.0-76-generic-pae #111-Ubuntu SMP Tue Jan 13 22:34:29 UTC 2015 i686
12:22:26     INFO -  CPU: x86
12:22:26     INFO -       GenuineIntel family 6 model 62 stepping 4
12:22:26     INFO -       1 CPU
12:22:26     INFO -  Crash reason:  SIGSEGV
12:22:26     INFO -  Crash address: 0x0
12:22:26     INFO -  Process uptime: not available
12:22:26     INFO -  Thread 18 (crashed)
12:22:26     INFO -   0  libxul.so!mozilla::dom::Blob::Release [File.cpp:056e578d03bb : 158 + 0x27]
12:22:26     INFO -      eip = 0xb1dcd6c3   esp = 0xa1afeef0   ebp = 0xa1afef28   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0x949ff1f0   edi = 0xb7101240   eax = 0xb4a51d86   ecx = 0xb60bb1a8
12:22:26     INFO -      edx = 0x00000000   efl = 0x00010282
12:22:26     INFO -      Found by: given as instruction pointer in context
12:22:26     INFO -   1  libxul.so!mozilla::RefPtrTraits<mozilla::dom::File>::Release [RefPtr.h:056e578d03bb : 40 + 0x6]
12:22:26     INFO -      eip = 0xb1de4afc   esp = 0xa1afef30   ebp = 0xa1afef48   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0x00000000   edi = 0x8fa4579c
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -   2  libxul.so!nsTArray_Impl<RefPtr<mozilla::dom::File>, nsTArrayFallibleAllocator>::RemoveElementsAt [nsTArray.h:056e578d03bb : 557 + 0x5]
12:22:26     INFO -      eip = 0xb1de7fb9   esp = 0xa1afef50   ebp = 0xa1afef88   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0x00000000   edi = 0x8fa4579c
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -   3  libxul.so!nsTArray_Impl<RefPtr<mozilla::dom::File>, nsTArrayFallibleAllocator>::Clear [nsTArray.h:056e578d03bb : 1635 + 0xa]
12:22:26     INFO -      eip = 0xb1de8023   esp = 0xa1afef90   ebp = 0xa1afefa8   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa0c872c0   edi = 0xa0c87310
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -   4  libxul.so!mozilla::dom::GetFilesHelper::~GetFilesHelper [nsTArray.h:056e578d03bb : 872 + 0xd]
12:22:26     INFO -      eip = 0xb27ab77d   esp = 0xa1afefb0   ebp = 0xa1afefe8   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa0c872c0   edi = 0xa0c87310
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -   5  libxul.so!mozilla::dom::GetFilesHelperParent::~GetFilesHelperParent [GetFilesHelper.cpp:056e578d03bb : 613 + 0x9]
12:22:26     INFO -      eip = 0xb27adc9c   esp = 0xa1afeff0   ebp = 0xa1aff028   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa0c872c0   edi = 0xa1aff00c
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -   6  libxul.so!mozilla::dom::GetFilesHelperParent::~GetFilesHelperParent [GetFilesHelper.cpp:056e578d03bb : 616 + 0x9]
12:22:26     INFO -      eip = 0xb27adcce   esp = 0xa1aff030   ebp = 0xa1aff048   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa0c872c0   edi = 0xa0c872c4
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -   7  libxul.so!mozilla::Runnable::Release [nsThreadUtils.cpp:056e578d03bb : 35 + 0x10]
12:22:26     INFO -      eip = 0xb1224f53   esp = 0xa1aff050   ebp = 0xa1aff088   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa0c872c0   edi = 0xa0c872c4
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -   8  libxul.so!nsCOMPtr<nsIRunnable>::~nsCOMPtr [nsCOMPtr.h:056e578d03bb : 404 + 0x9]
12:22:26     INFO -      eip = 0xb1191298   esp = 0xa1aff090   ebp = 0xa1aff0a8   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa1aff0f0   edi = 0xb60bef90
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -   9  libxul.so!nsThreadPool::Run [nsThreadPool.cpp:056e578d03bb : 226 + 0xc]
12:22:26     INFO -      eip = 0xb1200624   esp = 0xa1aff0b0   ebp = 0xa1aff118   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa3186580   edi = 0xb60bef90
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -  10  libxul.so!nsThread::ProcessNextEvent [nsThread.cpp:056e578d03bb : 1059 + 0x14]
12:22:26     INFO -      eip = 0xb11fe404   esp = 0xa1aff120   ebp = 0xa1aff198   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa4439ba0   edi = 0xb60bef98
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -  11  libxul.so!NS_ProcessNextEvent [nsThreadUtils.cpp:056e578d03bb : 290 + 0x10]
12:22:26     INFO -      eip = 0xb1226367   esp = 0xa1aff1a0   ebp = 0xa1aff1d8   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa31c5340   edi = 0xa448e820
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -  12  libxul.so!mozilla::ipc::MessagePumpForNonMainThreads::Run [MessagePump.cpp:056e578d03bb : 338 + 0xc]
12:22:26     INFO -      eip = 0xb15b93cf   esp = 0xa1aff1e0   ebp = 0xa1aff228   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa31c5340   edi = 0xa448e820
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -  13  libxul.so!MessageLoop::RunInternal [message_loop.cc:056e578d03bb : 232 + 0x14]
12:22:26     INFO -      eip = 0xb1593aca   esp = 0xa1aff230   ebp = 0xa1aff258   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa448e820   edi = 0xa31c5340
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -  14  libxul.so!MessageLoop::Run [message_loop.cc:056e578d03bb : 225 + 0x8]
12:22:26     INFO -      eip = 0xb1593af0   esp = 0xa1aff260   ebp = 0xa1aff288   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa448e820   edi = 0xa31c5340
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -  15  libxul.so!nsThread::ThreadFunc [nsThread.cpp:056e578d03bb : 459 + 0x9]
12:22:26     INFO -      eip = 0xb11ff0d5   esp = 0xa1aff290   ebp = 0xa1aff2d8   ebx = 0xb60bb1a8
12:22:26     INFO -      esi = 0xa448e820   edi = 0xa31c5340
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -  16  libnspr4.so!_pt_root [ptthread.c:056e578d03bb : 216 + 0x7]
12:22:26     INFO -      eip = 0xb73c7736   esp = 0xa1aff2e0   ebp = 0xa1aff328   ebx = 0xb73d8468
12:22:26     INFO -      esi = 0x00000000   edi = 0xa31ac600
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -  17  libpthread-2.15.so + 0x6d4c
12:22:26     INFO -      eip = 0xb76cbd4c   esp = 0xa1aff330   ebp = 0xa1aff428   ebx = 0xb76dcff4
12:22:26     INFO -      esi = 0x00000000   edi = 0x003d0f00
12:22:26     INFO -      Found by: call frame info
12:22:26     INFO -  18  libc-2.15.so + 0xeeb8e
12:22:26     INFO -      eip = 0xb74ccb8e   esp = 0xa1aff430   ebp = 0x00000000
12:22:26     INFO -      Found by: previous frame's frame pointer
Status: RESOLVED → REOPENED
status-firefox51: fixed → affected
Flags: needinfo?(amarchesini)
Resolution: FIXED → ---
Target Milestone: mozilla51 → ---
Summary: Intermittent dom/filesystem/compat/tests/test_formSubmission.html | application crashed [@ mozilla::dom::ContentParent::Release] → Intermittent dom/filesystem/compat/tests/test_formSubmission.html | application crashed [@ mozilla::dom::ContentParent::Release] or [@ mozilla::dom::Blob::Release]
Attached patch getFiles_crash3.patch (obsolete) — — Splinter Review
Flags: needinfo?(amarchesini)
Attachment #8791502 - Flags: review?(bugs)
Comment on attachment 8791502 [details] [diff] [review]
getFiles_crash3.patch


> 
>   nsTArray<RefPtr<Promise>> mPromises;
>   nsTArray<RefPtr<GetFilesCallback>> mCallbacks;
>+  Sequence<RefPtr<File>> mFiles;
>+  nsIGlobalObject* mGlobal;
Why is this one member variable raw pointer but others aren't?
I think this should be nsCOMPtr and then just assign null to that in Run().

If you're worried that Run is never called or something, then having just one member variable as raw pointer doesn't help.
You could explicitly in dtor leak the values hold by member variables at that time.
Attachment #8791502 - Flags: review?(bugs) → review-
Attached patch getFiles_crash3.patch (obsolete) — — Splinter Review
Attachment #8791502 - Attachment is obsolete: true
Attachment #8791643 - Flags: review?(bugs)
Attachment #8791643 - Attachment is obsolete: true
Attachment #8791643 - Flags: review?(bugs)
Attachment #8791653 - Flags: review?(bugs)
Summary: Intermittent dom/filesystem/compat/tests/test_formSubmission.html | application crashed [@ mozilla::dom::ContentParent::Release] or [@ mozilla::dom::Blob::Release] → Intermittent dom/filesystem/compat/tests/test_formSubmission.html,test_worker_basic.html | application crashed [@ mozilla::dom::ContentParent::Release] or [@ mozilla::dom::Blob::Release]
Duplicate of this bug: 1288612
Duplicate of this bug: 1302880
Comment on attachment 8791653 [details] [diff] [review]
getFiles_crash3.patch

You shouldn't have NS_IF_RELEASE(mGlobal); when mGlobal is nsCOMPtr.
mGlobal = nullptr;

With that, r+.
Attachment #8791653 - Flags: review?(bugs) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b707a4dd056
GetFilesHelper must release mFiles and mGlobal on the main-thread, r=smaug
https://hg.mozilla.org/mozilla-central/rev/7b707a4dd056
Status: REOPENED → RESOLVED
Closed: 3 years ago → 3 years ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Looks like the third time was the charm :). Can you please request Beta approval on this when you get a chance (assuming that Aurora50 gets uplifted to Beta before this'll get approved). Might be worth folding these into one patch for uplift too.
Flags: needinfo?(amarchesini)
Here's a rollup patch. Please do the Beta approval request if it looks good to you, Andrea :)
Attachment #8794213 - Flags: feedback?(amarchesini)
Attachment #8794213 - Flags: feedback?(amarchesini) → feedback+
Comment on attachment 8794213 [details] [diff] [review]
roll-up patch for beta uplift

Approval Request Comment
[Feature/regressing bug #]: Entries API
[User impact if declined]: a crash can occur when GetFilesHelper is released in a non main-thread
[Describe test coverage new/current, TreeHerder]: green on try.
[Risks and why]: not really: we just force the release of internal members in the main-thread.
[String/UUID change made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8794213 - Flags: approval-mozilla-beta?
Attachment #8794213 - Flags: approval-mozilla-aurora?
Comment on attachment 8794213 [details] [diff] [review]
roll-up patch for beta uplift

Fixes an intermittent failure, Beta50+

(Cleared aurora nom as this landed before the merge day and is already on 51)
Attachment #8794213 - Flags: approval-mozilla-beta?
Attachment #8794213 - Flags: approval-mozilla-beta+
Attachment #8794213 - Flags: approval-mozilla-aurora?
You need to log in before you can comment on or make changes to this bug.