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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Gregor Wagner [:gwagner]]

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=...)

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

Thanks for reporting this.

I'm taking this bug to follow up.

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

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

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5041068d4180

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch: Suppress assertion check in ~ErrorResult() if no exception to be thrown.

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

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

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!

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=df0523858c0a

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

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

Comment 8

[Edgar Chen [:edgar]]

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.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/bddfb9cc0a78

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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