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

Status: RESOLVED FIXED

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

Description

[Julian Seward [:jseward]]

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.

Comment 1

[Jean-Yves Avenard [:jya]]

John knows best here

Comment 2

[John Lin [:jhlin][:jolin]]

Looks like it failed to allocate shared memory for IPC. Will add error handling code for it.

Comment 3

[John Lin [:jhlin][:jolin]]

RemoteDataDecoder in central and aurora are so diverged that uplifting is not going to work. Will generate 2 patch sets instead.

Comment 4

[John Lin [:jhlin][:jolin]]

Attachment: Bug 1340582 - handle shared memory allocation failure.

Review commit: https://reviewboard.mozilla.org/r/113818/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/113818/

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

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.

Comment 6

[John Lin [:jhlin][:jolin]]

Comment on attachment 8839066 [details]
Bug 1340582 - handle shared memory allocation failure.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/113818/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf67ad546e37
handle shared memory allocation failure. r=jchen

Comment 8

[John Lin [:jhlin][:jolin]]

Attachment: Bug 1340582 - handle shared memory allocation failure: the aurora version.

No idea how to sent this to mozreview so there it is.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bf67ad546e37

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

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);

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a33bae98aee
fix exception logging. r=jchen

Comment 12

[John Lin [:jhlin][:jolin]]

Attachment: Bug 1340582 - handle shared memory allocation failure: the 2nd aurora version

Address jchen's comment and carry r+.

Comment 13

[John Lin [:jhlin][:jolin]]

Try result of the patch for aurora: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab3052acb7f3ca9ce9f3112ee76fe494aa020c28&selectedJob=79542086

Comment 14

[John Lin [:jhlin][:jolin]]

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

Comment 15

[Gerry Chang [:gchang]]

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+.

Comment 16

[John Lin [:jhlin][:jolin]]

(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.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0a33bae98aee

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/491f9e0666e7