1031227 - System Message Manager try to access to a null variable

Status: RESOLVED DUPLICATE of bug 1035074

(Core :: DOM: Device Interfaces, defect)

Description

[Borja Salguero [:borjasalguero]]

Checked with Flame, v2.0. STR:
- Use an App with push.
- Check that Im receiving push.
- Disconnect/Connect Wifi

EXPECTED:
- New push messages are received

CURRENTLY:
- When a new push message is received, I get the following error:

E/GeckoConsole( 1243): [JavaScript Error: "this._dispatchers is null" {file: "jar:file:///system/b2g/omni.ja!/components/SystemMessageManager.js" line: 225}]


I'm retrieving the same bug when the screen is switched off due to the timeout of the screen.

Comment 1

[Borja Salguero [:borjasalguero]]

Fernando, could you take a look? Thanks!

Comment 2

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Borja Salguero (this week part-time in Gaia) [:borjasalguero] from comment #1)
> Fernando, could you take a look? Thanks!

Sure. Can you provide some logs?

Comment 3

[Germán Toro del Valle (:gtorodelvalle)]

Hi guys, the system message issue could also be affecting bug 1031257. But this is only the tip of the iceberg :O I am observing all kind of issues in the Dialer/Call Screen (calls not established, no need to talk of more complex scenarios such as call waiting of conference calls) where I am getting the error Borja mentions over and over again :-)

Comment 4

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Renaming, since it's not Push bug but System Message

Comment 5

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Attachment: Traces of System Message Manager

German send me this traces. Thank you German

You can reproduce this with two phone calls, the second call have the issue.

Comment 6

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Attachment: Simple patch to avoid the crash

I'll continue looking for the cause why this variable is null into these cases

Comment 7

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Attachment: Simple patch to avoid the crash

Sorry, I ate an 's' in the variable name :p

Comment 8

[Antonio Manuel Amaya Calvo (:amac)]

Comment on attachment 8448710 [details] [diff] [review]
Simple patch to avoid the crash

If this._dispatchers is null then the object has been destroyed already... I guess what's happening is that the parent process is answering some lingering request too late for them to be attended. And I think it would be better just to null out this._manifestURL and this._pageURL also on the uninit function (and that way this would just return not doing anything).

Comment 9

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Next week I'll be out, so moving to Antonio

Comment 10

[bhavana bajaj [:bajaj]]

Ni Borja, is this related to loop ? What app are we exactly using ? Can you help understand the end user impact ?

Comment 11

[Borja Salguero [:borjasalguero]]

Hi Bhavana. This is not directly related with Loop. This is related with System Messages, and as Loop is using Push Notifications (and for notifying them Gecko is using System Messages) we are affected about this issue.

This bug is affecting every protocol & API based on System Message, for example related with incoming call/messages (check https://bugzilla.mozilla.org/show_bug.cgi?id=1031227#c3), Push Notifications, IAC...

I hope it helps!

Comment 12

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V1. Nullify the manifestURL and the pageURL on the destructor

The problem happens when:
 * A child process loads a page that sets a message handler.
 * That sends a IPC message to the parent process asking for the pending messages
 * Before the parent answer, the child process navigates away from the page that set the handler. MessageManager is destructed
 * And when the parent answers with the SystemMessageManager:GetPendingMessages:Return message, the child process try to process it on a object that has been uninitialized.

I think the easiest solution is to nullify correctly the object on the destructor. This will cause the message to be ignored.

By the way, this will fix the crash but unless the crash is stopping some other process, it will not fix the original problem, still trying to reproduce that.

Comment 13

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #12)
> Created attachment 8452280 [details] [diff] [review]
> 
> By the way, this will fix the crash but unless the crash is stopping some
> other process, it will not fix the original problem, still trying to
> reproduce that.

Well, according to Borja, this actually fixed his problems with Push also... TBH I would like to know why this failing was making push fail too, but not going to look a gift horse in the mouth.

Comment 14

[Gene Lian (I already quit Mozilla)]

This issue has been reported and a better solution has been proposed over there.

Hi Antonio, I'd like to close this one as a dup. Please reopen it if you disagree. Thanks!

Comment 15

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8452280 [details] [diff] [review]
V1. Nullify the manifestURL and the pageURL on the destructor

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

Bug 1035074 has already has a relatively better patch.

Comment 16

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Gene Lian [:gene] (needinfo? encouraged) from comment #15)
> Comment on attachment 8452280 [details] [diff] [review]
> V1. Nullify the manifestURL and the pageURL on the destructor
> 
> Review of attachment 8452280 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Bug 1035074 has already has a relatively better patch.

That was Fernando proposed patch also :). I liked it better clearing the object on the destructor so no special code for the strange case is needed on the message handler, but I'm ok with either, thanks!