Closed Bug 1340582 Opened 7 years ago Closed 7 years ago

Crash in java.lang.NullPointerException: Null shared memory buffer at org.mozilla.gecko.mozglue.SharedMemBuffer.nativeReadFromDirectBuffer(Native Method)

Categories

(Core :: Audio/Video, defect, P1)

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- unaffected
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: jseward, Assigned: jhlin)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-ecb993ec-d084-45bb-8a43-9267b2170217.
=============================================================

This is ranked #1 in the Android crashes for Aurora 20170216084122.
John knows best here
Flags: needinfo?(jolin)
Looks like it failed to allocate shared memory for IPC. Will add error handling code for it.
Assignee: nobody → jolin
Flags: needinfo?(jolin)
RemoteDataDecoder in central and aurora are so diverged that uplifting is not going to work. Will generate 2 patch sets instead.
Comment on attachment 8839066 [details]
Bug 1340582 - handle shared memory allocation failure.

https://reviewboard.mozilla.org/r/113818/#review115444

::: mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java:180
(Diff revision 1)
>              if (sample == null) {
>                  return false;
>              }
>              mRemote.queueInput(sample);
>              sample.dispose();
> -        } catch (RemoteException e) {
> +        } catch (Exception e) {

It's better to specify the individual exceptions, e.g.

    } catch (RemoteException|IOException e) {

But `Exception` is okay if you think the code can throw other exceptions that we don't know about right now.

::: mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java:181
(Diff revision 1)
>                  return false;
>              }
>              mRemote.queueInput(sample);
>              sample.dispose();
> -        } catch (RemoteException e) {
> +        } catch (Exception e) {
>              e.printStackTrace();

Remove this line.

::: mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java:184
(Diff revision 1)
>              sample.dispose();
> -        } catch (RemoteException e) {
> +        } catch (Exception e) {
>              e.printStackTrace();
>              Log.e(LOGTAG, "fail to input sample: size=" + info.size +
>                      ", pts=" + info.presentationTimeUs +
>                      ", flags=" + Integer.toHexString(info.flags));

Add `, e);` to the `Log.e` call. This will automatically print the stack trace.
Attachment #8839066 - Flags: review?(nchen) → review+
Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf67ad546e37
handle shared memory allocation failure. r=jchen
No idea how to sent this to mozreview so there it is.
Attachment #8839815 - Flags: review?(nchen)
https://hg.mozilla.org/mozilla-central/rev/bf67ad546e37
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8839815 [details] [diff] [review]
Bug 1340582 - handle shared memory allocation failure: the aurora version.

Review of attachment 8839815 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java
@@ +181,4 @@
>              Log.e(LOGTAG, "fail to input sample: size=" + info.size +
>                      ", pts=" + info.presentationTimeUs +
> +                    ", flags=" + Integer.toHexString(info.flags) +
> +                    ", e=" + e);

"e" should be a separate argument to `Log.e`, i.e.

> Log.e(LOGTAG, "fail to input sample: size=" + info.size +
>               ", pts=" + info.presentationTimeUs +
>               ", flags=" + Integer.toHexString(info.flags), e);
Attachment #8839815 - Flags: review?(nchen) → review+
Address jchen's comment and carry r+.
Attachment #8839815 - Attachment is obsolete: true
Attachment #8840305 - Flags: review+
Comment on attachment 8840305 [details] [diff] [review]
Bug 1340582 - handle shared memory allocation failure: the 2nd aurora version

Approval Request Comment
[Feature/Bug causing the regression]: 1340582
[User impact if declined]: crash when device cannot allocate shared memory
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low risk
[Why is the change risky/not risky?]: more robust error handling
[String changes made/needed]: none
Attachment #8840305 - Flags: approval-mozilla-aurora?
Priority: -- → P1
Comment on attachment 8840305 [details] [diff] [review]
Bug 1340582 - handle shared memory allocation failure: the 2nd aurora version

Hi John,
The [Feature/Bug causing the regression] in template should not be the bug itself. We can take the patch this time in order to fix the crash. Please fill the correct bug/feature in the template next time for better judgment. Aurora53+.
Flags: needinfo?(jolin)
Attachment #8840305 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Gerry Chang [:gchang] from comment #15)
> Comment on attachment 8840305 [details] [diff] [review]
> Bug 1340582 - handle shared memory allocation failure: the 2nd aurora version
> 
> Hi John,
> The [Feature/Bug causing the regression] in template should not be the bug
> itself. We can take the patch this time in order to fix the crash. Please
> fill the correct bug/feature in the template next time for better judgment.
> Aurora53+.

 My apology. That should be bug 1295106. Thanks a lot.
Flags: needinfo?(jolin)
You need to log in before you can comment on or make changes to this bug.