Gallery opened in the background crashes when recording a video

RESOLVED FIXED in Firefox 21

Status

Firefox OS
General
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: ttaubert, Assigned: Ben Turner (not reading bugmail, use the needinfo flag!))

Tracking

({crash})

unspecified
B2G C4 (2jan on)
ARM
Gonk (Firefox OS)
crash

Firefox Tracking Flags

(blocking-basecamp:+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed)

Details

(Whiteboard: [b2g-crash], crash signature)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
While trying to reproduce bug 827311 I hit a crash quite often but not always when recording a video with the Gallery opened in the background.

[Child 2325] ###!!! ABORT: [PBlobStreamChild] abort()ing as a result: file /home/tim/workspace/b2g-desktop/objdir-gecko-unagi/ipc/ipdl/PBlobStreamChild.cpp, line 364

Program received signal SIGSEGV, Segmentation fault.
0x4102e6aa in mozalloc_abort (msg=<value optimized out>) at /home/tim/workspace/b2g-desktop/memory/mozalloc/mozalloc_abort.cpp:30
30	    MOZ_CRASH();
(gdb) bt
#0  0x4102e6aa in mozalloc_abort (msg=<value optimized out>) at /home/tim/workspace/b2g-desktop/memory/mozalloc/mozalloc_abort.cpp:30
#1  0x40bb6140 in Abort (aSeverity=<value optimized out>, aStr=0x410a4286 "[PBlobStreamChild] abort()ing as a result", aExpr=<value optimized out>, aFile=<value optimized out>, aLine=364)
    at /home/tim/workspace/b2g-desktop/xpcom/base/nsDebugImpl.cpp:423
#2  NS_DebugBreak_P (aSeverity=<value optimized out>, aStr=0x410a4286 "[PBlobStreamChild] abort()ing as a result", aExpr=<value optimized out>, aFile=<value optimized out>, aLine=364)
    at /home/tim/workspace/b2g-desktop/xpcom/base/nsDebugImpl.cpp:380
#3  0x40b1fa06 in mozilla::dom::PBlobStreamChild::FatalError (this=<value optimized out>, msg=<value optimized out>) at /home/tim/workspace/b2g-desktop/objdir-gecko-unagi/ipc/ipdl/PBlobStreamChild.cpp:364
#4  0x40b202c4 in mozilla::dom::PBlobStreamChild::OnMessageReceived (this=0x43870480, __msg=...) at /home/tim/workspace/b2g-desktop/objdir-gecko-unagi/ipc/ipdl/PBlobStreamChild.cpp:215
#5  0x40b29cb2 in mozilla::dom::PContentChild::OnMessageReceived (this=0x4191b618, __msg=...) at /home/tim/workspace/b2g-desktop/objdir-gecko-unagi/ipc/ipdl/PContentChild.cpp:2182
#6  0x40ac60e0 in mozilla::ipc::AsyncChannel::OnDispatchMessage (this=0x4191b620, msg=...) at /home/tim/workspace/b2g-desktop/ipc/glue/AsyncChannel.cpp:473

Updated

6 years ago
Severity: normal → critical
Crash Signature: [@ mozalloc_abort | NS_DebugBreak_P | mozilla::dom::PBlobStreamChild::FatalError]

Updated

6 years ago
Assignee: nobody → hub
Tim, can you provide STR. In particular, which app is crashing? Which app is in the forground when an app is crashing? Are there webactivities involved here by any chance? I.e. how are you opening these apps? From the homescreen?
Keywords: steps-wanted
(Reporter)

Comment 2

6 years ago
The Gallery app crashes while it's in the background. The foreground app is the Camera. You sometimes need a couple of tries. If the Gallery doesn't crash while recording a video, close the Camera app (close, not hide) and re-open the Camera and try again.
Tim, what's the user experience here when the background gallery app crashes? IOW, can we just let it crash in the background and move on, or does it crashing significantly hurt the user experience?
(Reporter)

Comment 4

6 years ago
So far the Gallery always crashed when it was completely hidden. So if the user remembers that it was open they might notice something. I doubt anybody's experience will be affected by this.
Users will see a notification of a crash while they're doing something unrelated.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #5)
> Users will see a notification of a crash while they're doing something
> unrelated.

No, I recalled that by design the user would only see a banner when the foreground app is crashed,
a crashing background app is closed quietly.

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/window_manager.js#L1547
I did get a message telling me something crashed. There was no application name or anything. Definitely the user would have seen something.
(In reply to Hub Figuiere [:hub] from comment #7)
> I did get a message telling me something crashed. There was no application
> name or anything. Definitely the user would have seen something.

Sounds like a bug, and it's a dup...

Updated

6 years ago
blocking-basecamp: ? → +
Created attachment 699777 [details]
Store dummy file as a dotfile so it is ignored by the gallery
Attachment #699777 - Flags: review?(hub)
Assignee: hub → bent.mozilla
Comment on attachment 699777 [details]
Store dummy file as a dotfile so it is ignored by the gallery

This bug is going to be fixed at the source (its a problem in platform while handling the dummy file)

But this works around triggering it by creating the dummy file as a dotfile, therefore it creates the necessary directories but is ignored by the gallery.
Attachment #699777 - Flags: review?(hub) → review?(dflanagan)
Comment on attachment 699777 [details]
Store dummy file as a dotfile so it is ignored by the gallery

The patch looks good to me.

Where was the crash actually happening?  In the get_video_rotation file?  I'd like to fix the underlying crash, too. 

Land this workaround for v1, but file a bug about the crashy code and assign it to me, please?
Attachment #699777 - Flags: review?(dflanagan) → review+
As far as I understand the crash isnt anything inherently wrong in the gaia code, it just triggers a bug in platform due to the attempt by the gallery to play / read the metadata of the dummy file while it is being deleted
Gaia workaround is merged in: https://github.com/mozilla-b2g/gaia/commit/0437618f969fa741272298dabe6ada957f4495e3

This could be marked as fixed and the underlying bug opened seperately, or not
I did a little digging, and it looks to me like what is happening (before this patch is applied) is that mediadb is getting an event about the dummy file, but before it can insert that file into its database, the file has been deleted. Most of the time for me this results in a benign error message in logcat.  I can't reproduce the crash reliably, so it seems like a timing-related issue.

MediaDB may not be robust in the case of files being created and then quickly deleted.  It still shouldn't crash, though. Maybe that is a device storage bug.  I think something landed recently that affects file deletion. I wonder if that could have fixed whatever was underlying this?

Doug: do you want to keep this bug open to investigate the underlying issue? If not, would you close it?
Flags: needinfo?(doug.turner)
Bent is working on a gecko patch. Clearing the needinfo for doug.
Flags: needinfo?(doug.turner)
Created attachment 700267 [details] [diff] [review]
Patch, v1

This should fix the crash. We aggressively duplicate file handles now.
Attachment #700267 - Flags: review?(jones.chris.g)
Created attachment 700286 [details] [diff] [review]
Patch, v1.1

Now without main-process favoritism.
Attachment #700267 - Attachment is obsolete: true
Attachment #700267 - Flags: review?(jones.chris.g)
Attachment #700286 - Flags: review?(jones.chris.g)
Comment on attachment 700286 [details] [diff] [review]
Patch, v1.1

>diff --git a/ipc/glue/FileDescriptor.cpp b/ipc/glue/FileDescriptor.cpp

> FileDescriptor::PickleType
> FileDescriptor::ShareTo(const FileDescriptor::IPDLPrivate&,
>-                        FileDescriptor::ProcessHandle aOtherProcess) const
>+                        FileDescriptor::ProcessHandle aOtherProcess)
> {

Would be cleaner as,

 if (mHandle == INVALID_HANDLE) {
   return base::FileDescriptor();
 }
 // other stuff here assuming mHandle != INVALID_HANDLE

>+#else // XP_WIN
>+  if (mHandle != INVALID_HANDLE) {
>+    // The caller will close the file descriptor once it is done using it, so
>+    // we don't need to manually close in the destructor.
>+    mMustCloseHandle = false;
>+    return base::FileDescriptor(mHandle, /* auto_close */ true);

I don't like the fact that ShareTo() mutates FileDescriptor.  Let's
continue to do the simple and slow-ish extra dup.

r=me with that
Attachment #700286 - Flags: review?(jones.chris.g) → review+
After discussion I didn't address the first comment. Second addressed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3b6bc4b2590
As well as M2, also xpcshell failures (in case you hadn't seen them :-)):
https://tbpl.mozilla.org/php/getParsedLog.php?id=18670198&tree=Mozilla-Inbound
Created attachment 700414 [details] [diff] [review]
Patch v1.2

This handles the receiving-side better.
Attachment #700286 - Attachment is obsolete: true
Attachment #700414 - Flags: review?(jones.chris.g)
Comment on attachment 700414 [details] [diff] [review]
Patch v1.2

r=me with IRL comments addressed.
Attachment #700414 - Flags: review?(jones.chris.g) → review+
Backed out on inbound for what appears to be failing all tests on OS X. Either this patch or the one for bug 828870 is the cause.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9cb5b72a226d
Please reland this when the tree reopens. It turned out to be an infra issue, sorry. https://bugzilla.mozilla.org/show_bug.cgi?id=829169

Updated

5 years ago
Keywords: steps-wanted
This needs a bit of work before it can land on b2g.
What work is needed?  The patch applies cleanly ... ;).

Comment 30

5 years ago
jst, are you landing this?
marking resolved fixed per new jst rules.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
status-firefox19: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → fixed
Target Milestone: --- → B2G C4 (2jan on)
You need to log in before you can comment on or make changes to this bug.