793831 - [b2g-ril] RIL crashes on desktop when trying to write to non-open socket

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

At some point during recent gaia development, opening the Setting application causes RIL to try to write some data. On desktop B2G, since no RIL socket exists, this crashes due to not having validity checks for the socket.

Comment 1

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - Add socket validity checks to RIL IPC

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 664193 [details] [diff] [review]
Patch 1 (v1) - Add socket validity checks to RIL IPC

>diff --git a/ipc/ril/Ril.cpp b/ipc/ril/Ril.cpp

>+void RilWriteTask::Run()
>+{
>+    if(sClient->mSocket.get() <= 0) {

< 0.  "0" is a valid fd.

r=me with that.

Comment 3

[Philipp von Weitershausen [:philikon]]

(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #0)
> At some point during recent gaia development, opening the Setting
> application causes RIL to try to write some data.

I was going to ask what data this was, but then I discovered that it's probably

  navigator.mozMobileConnection.getCardLock('pin');

which makes perfect sense. Just posting this here in case anybody is wondering why Settings would talk to the radio.

Comment 4

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Try result: https://tbpl.mozilla.org/?tree=Try&rev=7b837b196ca1

https://hg.mozilla.org/integration/mozilla-inbound/rev/c88496e8454f

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Backed out, sorry

https://hg.mozilla.org/integration/mozilla-inbound/rev/295fcca29ff4

Comment 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v2) - Add socket validity checks to RIL IPC

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 666099 [details] [diff] [review]
Patch 1 (v2) - Add socket validity checks to RIL IPC

For some reason, evaluation of the ScopedClose without a .get() was doing weird things in the OpenSocket function. Fixed that. Now deals gracefully with socket handoff.

Also fixed an intermittent crash on MOZ_ASSERT of the IOLoop on shutdown by changing it to a task cancel and exit. Apparently our retry task doesn't die until after the IOLoop does?

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 666099 [details] [diff] [review]
Patch 1 (v2) - Add socket validity checks to RIL IPC

r=me if you tested on device.  If you break RIL, I break your face ;).

Comment 9

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Called myself from the otoro with patch applied. 

https://hg.mozilla.org/integration/mozilla-inbound/rev/1983304d2e60

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1983304d2e60