995194 - Assertion failure when stopping a Loop call

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Mark Banner (:standard8)]

I've just updated loop-ui-initial to the latest mozilla/gecko-dev master. In testing, when I hang up a call, I get a crash:

Assertion failure: !cx->isExceptionPending(), at /Users/mark/loop/gecko-merge-master/js/src/jscntxtinlines.h:242

Catching in a debugger and doing "call DumpJSStack()" I get:

Assertion failure: !cx->isExceptionPending(), at /Users/mark/loop/gecko-merge-master/js/src/jscntxtinlines.h:242
0 anonymous(cons = "The publisher 48a50b16-a2f4-4bcb-8ff5-266af45770c4 is trying to unpublish from a session 2_MX40NDY2OTEwMn5-RnJpIEFwciAxMSAwNjoxMzoyOCBQRFQgMjAxNH4wLjg0NjUzNzc3flB- it is not attached to (no this.session)") ["chrome://browser/content/loop/shared/libs/sdk.js":905]
    this = [object Object]
1 anonymous(session = [object Object], reason = "unpublished") ["chrome://browser/content/loop/shared/libs/sdk.js":14453]
    this = [object Object]
2 anonymous(publisher = [object Object]) ["chrome://browser/content/loop/shared/libs/sdk.js":16740]
    this = [object Object]
3 anonymous(event = [object Object]) ["chrome://browser/content/loop/shared/js/views.js":115]
    this = [object Object]
4 anonymous() ["chrome://browser/content/loop/shared/libs/sdk.js":1167]
    this = [object Window]
5 anonymous(event = [object MessageEvent]) ["chrome://browser/content/loop/shared/libs/sdk.js":1060]
    this = [object Window]

This seemed to be caused within the sdk. Knowing that we've recently added some unpublish lines, I played around and found an issue in sessionDisconnected - I think sessionDisconnected must unpublish the stream (it would be silly not to), and so when we get the notification that the disconnection has finished, trying to unpublish it again is not a good idea.

What I don't understand is why this is now causing an assertion in gecko. The js code seems to indicate it is already throwing an exception when another one comes along, but I couldn't see where.

For now, the PR that I'll attach in a moment fixes the issue.

Comment 1

[Mark Banner (:standard8)]

Attachment: Don't unpublish

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Reviewed and merged.  Leaving open in case there's more you want to do here.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

(Eg either file bugs against the SDK, Gecko, or both, since it shouldn't be possible to cause assertions from JS code like this).

Comment 4

[:shell escalante]

Root cause was solved, this is a holder for future issues / follow-up bugs and moved from MLP to MVP desktop client bug.

Comment 5

[Mark Banner (:standard8)]

https://hg.mozilla.org/projects/elm/rev/365c4fdd390b

Comment 6

[Mark Banner (:standard8)]

This completed a while ago. I can't reproduce these assertions atm, hence closing. If we get more in future, we can file those appropriately.

Comment 7

[Mark Banner (:standard8)]

https://hg.mozilla.org/mozilla-central/rev/365c4fdd390b

Comment 8

[u279076]

Looks like this landed with tests. Does this need QA testing?

Comment 9

[Mark Banner (:standard8)]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #8)
> Looks like this landed with tests. Does this need QA testing?

This would be covered by general smoketesting in debug mode, or the functional tests running in debug mode once we've got that far.

The main issue here was the crash that was happening.

Comment 10

[u279076]

Deprioritizing this for QA testing. I'll make sure running through our smoketests in Debug mode is done at least once in Aurora.