Closed Bug 912517 Opened 6 years ago Closed 6 years ago

[Message] Crash occuring in MMS when killing the application while the MMS is sending

Categories

(Firefox OS Graveyard :: RIL, defect, critical)

ARM
Gonk (Firefox OS)
defect
Not set
critical

Tracking

(blocking-b2g:leo+, firefox24 wontfix, firefox25 wontfix, firefox26 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

RESOLVED FIXED
blocking-b2g leo+
Tracking Status
firefox24 --- wontfix
firefox25 --- wontfix
firefox26 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed

People

(Reporter: leo.bugzilla.gaia, Assigned: leo.bugzilla.gaia)

References

Details

Attachments

(3 files, 3 obsolete files)

1. Title: Crash occuring in MMS when killing the application while the MMS is sending
2. Precondition: Should be able to send and receive messages
3. Tester's Action:  (1) Open SMS application
                     (2) Select New Message 
                     (3) Send MMS with multiple images/Audio/Video
                     (4) Wait for a sometime while sending 
                     (5) kill the application
4. Detailed Symptom (ENG.) :The device crashes when killing the application and relaunching it again
5. Expected:  The device should not crash in any scenario
6. Reproducibility: Y
1) Frequency Rate : 100%
blocking-b2g: --- → leo+
This issue was happening because even after the child process was killed we are trying to access the message contents (issue happens when accessing attachment).

We already have a check to see whether the content process is killed or not, but thats too late in this case.

To fix this issue we have to do the check little earlier.
Attached patch Proposed patch (obsolete) — Splinter Review
Check whether the child process is killed or not.
Assignee: nobody → leo.bugzilla.gaia
Attachment #799546 - Flags: feedback?(vyang)
Hi, Patrick,
How do think about this patch "attachment 799546 [details] [diff] [review]"?
Thanks.
Flags: needinfo?(pwang)
Attached file Crash report 1
Attaching the crash report, so it will be easier to understand the patch.
Attached file Crash report 2
This is a second crash that I received related to this same issue.

But in this one, the crash happens after NotifyMessageGot (instead of NotifyMessageSent).

To leo.bugzilla.gaia: Shouldn't the same patch be applied to NotifyMessageGot also?
Thanks Andre,

We missed that one.
We will attach new patch after adding it.
Attached patch Proposed patch (obsolete) — Splinter Review
Added more check based on Comment #5
Attachment #799546 - Attachment is obsolete: true
Attachment #799546 - Flags: feedback?(vyang)
Attachment #799960 - Flags: feedback?(vyang)
Attachment #799960 - Flags: feedback?(pwang)
Comment on attachment 799960 [details] [diff] [review]
Proposed patch

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

::: dom/mobilemessage/src/ipc/SmsParent.cpp
@@ +25,5 @@
>  #include "nsTArrayHelpers.h"
>  
> +#undef LOG_TAG
> +#define LOG_TAG "MMSCPP"
> +#include <utils/Log.h>

Please remove the log here and __android_log_print below.
Attachment #799960 - Flags: feedback?(pwang) → feedback+
Flags: needinfo?(pwang)
Comment on attachment 799960 [details] [diff] [review]
Proposed patch

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

Please also check mActorDestroyed in other SmsRequestParent::Notify* functions. Thanks
Component: Gaia::SMS → DOM: Device Interfaces
Product: Boot2Gecko → Core
(In reply to Patrick Wang [:kk1fff] from comment #9)
> Please also check mActorDestroyed in other SmsRequestParent::Notify*
> functions. Thanks

They have already checks in |SendReply|.
Comment on attachment 799960 [details] [diff] [review]
Proposed patch

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

::: dom/mobilemessage/src/ipc/SmsParent.cpp
@@ +594,5 @@
>    nsCOMPtr<nsIDOMMozMmsMessage> mms = do_QueryInterface(aMessage);
> +
> +  __android_log_print(ANDROID_LOG_INFO, "GeckoMMS", "[Sharaf] SmsRequestParent::NotifyMessageSent %d", mActorDestroyed);
> +
> +  NS_ENSURE_TRUE(!mActorDestroyed, NS_ERROR_FAILURE);

Please move this check before |do_QueryInterface|.

  NS_ENSURE_TRUE(!mActorDestroyed, NS_ERROR_FAILURE);

  nsCOMPtr<nsIDOMMozMmsMessage> mms = do_QueryInterface(aMessage);
  if (mms) {
    ...

@@ +626,5 @@
>  SmsRequestParent::NotifyMessageGot(nsISupports *aMessage)
>  {
>    nsCOMPtr<nsIDOMMozMmsMessage> mms = do_QueryInterface(aMessage);
> +
> +  NS_ENSURE_TRUE(!mActorDestroyed, NS_ERROR_FAILURE);

ditto.
Attachment #799960 - Flags: feedback?(vyang) → feedback+
Attached patch Patch for the issue (obsolete) — Splinter Review
Hi,
Please find the updated patch with the review comments.
Thanks,
Attachment #799960 - Attachment is obsolete: true
Attachment #800611 - Flags: review?(vyang)
Attachment #800611 - Flags: review?(pwang)
Comment on attachment 800611 [details] [diff] [review]
Patch for the issue

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

Thanks.
Attachment #800611 - Flags: review?(pwang) → review+
Attachment #800611 - Flags: review?(vyang) → review+
Comment on attachment 800611 [details] [diff] [review]
Patch for the issue

Sorry, just found this patch wasn't generated from hg and don't have information about the author.  We need email and user name of A human being to land this patch.
Attachment #800611 - Flags: review-
Attachment #800611 - Flags: review+
Hi,
Please find the updated patch with the format
Thanks,
Attachment #800611 - Attachment is obsolete: true
Attachment #800693 - Flags: review?(vyang)
Comment on attachment 800693 [details] [diff] [review]
Patch with hg format

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

Thank you!
Attachment #800693 - Flags: review?(vyang) → review+
https://hg.mozilla.org/mozilla-central/rev/8c7b45e8adc9
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Blocks: b2g-mms
Component: DOM: Device Interfaces → RIL
Product: Core → Boot2Gecko
Target Milestone: mozilla26 → ---
You need to log in before you can comment on or make changes to this bug.