MOZ_ASSERT(!mMightHaveUnreportedJSException) via IccContactToMozContact when importing contacts from SIM card

RESOLVED FIXED in Firefox 43

Status

Firefox OS
RIL
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: gwagner, Assigned: bevis)

Tracking

unspecified
FxOS-S6 (04Sep)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
On Aries device with debug gecko.

Program received signal SIGSEGV, Segmentation fault.
0xb3fb5146 in mozilla::ErrorResult::~ErrorResult (this=<optimized out>, 
    __in_chrg=<optimized out>) at ../../dist/include/mozilla/ErrorResult.h:58
58	    MOZ_ASSERT(!mMightHaveUnreportedJSException);
(gdb) bt
#0  0xb3fb5146 in mozilla::ErrorResult::~ErrorResult (this=<optimized out>, 
    __in_chrg=<optimized out>) at ../../dist/include/mozilla/ErrorResult.h:58
#1  0xb4a941f0 in mozilla::dom::icc::(anonymous namespace)::IccContactToMozContact (aCx=aCx@entry=0xb1fe53d0, aGlobal=..., aIccContact=<optimized out>, 
    aMozContact=aMozContact@entry=0xbed74800)
    at /Users/gregor/moz/ib2g/dom/icc/IccCallback.cpp:90
#2  0xb4a9441e in IccContactListToMozContactList (aContactList=..., aCount=15, 
    aContacts=0xaccf7088, aGlobal=..., aCx=0xb1fe53d0)
    at /Users/gregor/moz/ib2g/dom/icc/IccCallback.cpp:102
#3  mozilla::dom::icc::IccCallback::NotifyRetrievedIccContacts (
    this=0xacc9eda0, aContacts=0xaccf7088, aCount=15)
    at /Users/gregor/moz/ib2g/dom/icc/IccCallback.cpp:246
#4  0xb4a93158 in mozilla::dom::icc::IccRequestChild::Recv__delete__ (
    this=0xaccad160, aResponse=...)
    at /Users/gregor/moz/ib2g/dom/icc/ipc/IccChild.cpp:455
#5  0xb401dc12 in OnMessageReceived (msg__=..., this=0xaccad160)
    at PIccRequestChild.cpp:162
#6  mozilla::dom::icc::PIccRequestChild::OnMessageReceived (this=0xaccad160, 
    msg__=...) at PIccRequestChild.cpp:135
#7  0xb414b43a in mozilla::dom::PContentChild::OnMessageReceived (
    this=0xb1fbcc18, msg__=...) at PContentChild.cpp:5465
#8  0xb3fb5d40 in mozilla::ipc::MessageChannel::DispatchAsyncMessage (
    this=0xb1fbcc50, aMsg=...)
(Reporter)

Updated

3 years ago
Flags: needinfo?(btseng)
(Assignee)

Comment 1

3 years ago
Thanks for reporting this.

I'm taking this bug to follow up.
Assignee: nobody → btseng
Flags: needinfo?(btseng)
(Assignee)

Comment 2

3 years ago
Root cause is found. It's all about how to use ErrorResult properly:
1. If the ErrorResult is the result of a method in JS implementation, ErrorResult will be set with MightThrowJSException() if there is any possibility to throw an exception in the context even though the exception is thrown at that call [1].
2. In IccCallback.cpp [2], we should always have "return er.StealNSResult()" at the end of this function instead of "return NS_OK" to implicitly suppress the exception which is not required in our implementation.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/bindings/CallbackObject.cpp#261
[2] https://dxr.mozilla.org/mozilla-central/source/dom/icc/IccCallback.cpp#90
(Assignee)

Comment 4

3 years ago
Created attachment 8649799 [details] [diff] [review]
Patch: Suppress assertion check in ~ErrorResult() if no exception to be thrown.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=95fee3af8a73
(Assignee)

Comment 5

3 years ago
Comment on attachment 8649799 [details] [diff] [review]
Patch: Suppress assertion check in ~ErrorResult() if no exception to be thrown.

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

Hi Edgar,

May I have your review for this quick fix?

Thanks!
Attachment #8649799 - Flags: review?(echen)
(Assignee)

Comment 7

3 years ago
Created attachment 8650305 [details] [diff] [review]
(v2) Patch: Suppress assertion check in ~ErrorResult() if no exception to be thrown.

Per offline discussion,
we can have StealNSResult() always be invoked with NS_ENSURE_SUCCESS to suppress the assertion and we should also revise the use of ErrorResult in IccContact.cpp in the same way as well.
Attachment #8649799 - Attachment is obsolete: true
Attachment #8649799 - Flags: review?(echen)
Attachment #8650305 - Flags: review?(echen)

Comment 8

3 years ago
Comment on attachment 8650305 [details] [diff] [review]
(v2) Patch: Suppress assertion check in ~ErrorResult() if no exception to be thrown.

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

Thank you.
Attachment #8650305 - Flags: review?(echen) → review+

Updated

3 years ago
Blocks: 1114937
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bddfb9cc0a78
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S6 (04Sep)
You need to log in before you can comment on or make changes to this bug.