RIL does not reconnect on socket connection loss

RESOLVED FIXED in mozilla11

Status

()

Core
IPC
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: qdot, Assigned: qdot)

Tracking

unspecified
mozilla11
All
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

If the RIL for some reason cannot pick up a connection to the rilb2g socket, it does not current retry for connection. This needs to be fixed in the RIL thread.
Blocks: 699235
Created attachment 581010 [details] [diff] [review]
Adding reconnection capabilities to RIL IPC thread
Attachment #581010 - Flags: review?(jones.chris.g)

Comment 2

6 years ago
Put a space between comments and the comment slashes //[ ]comment, and also put the "return" on a new line. I am not fluent in IPC, so lets wait for r=chris, but looks good to me. Nice fix.
Created attachment 581050 [details] [diff] [review]
RIL Patch with comment/return fixes
Attachment #581010 - Attachment is obsolete: true
Attachment #581010 - Flags: review?(jones.chris.g)
Attachment #581050 - Flags: review?(jones.chris.g)
Blocks: 710489
No longer blocks: 699235
Comment on attachment 581050 [details] [diff] [review]
RIL Patch with comment/return fixes

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

>+void RilReconnectTask::Run() {

Nit:

void
RilReconnectTask::Run()
{

>+    if(sClient->OpenSocket()) {

Nit: "if ("

>@@ -221,6 +234,13 @@ RilClient::OnFileCanReadWithoutBlocking(int fd)
>             int ret = read(fd, mIncoming->mData, 1024);
>             if (ret <= 0) {
>                 LOG("Cannot read from network, error %d\n", ret);
>+                //At this point, assume that we can't actually access
>+                //the socket anymore, and start a reconnect loop.

Nit: Space after "//".

>+                mIncoming.forget();
>+                mReadWatcher.StopWatchingFileDescriptor();
>+                mWriteWatcher.StopWatchingFileDescriptor();
>+                close(mSocket.mFd);

I would prefer if this were packaged into a "*State" kind of struct,
so that it could all be reset as a group, but this is OK for now.

Looks good, r=me with nits picked.
Attachment #581050 - Flags: review?(jones.chris.g) → review+
Created attachment 581809 [details] [diff] [review]
Post-Review RIL Patch with nits picked
Attachment #581050 - Attachment is obsolete: true
Keywords: checkin-needed
Blocks: 711315
Pushed (with the wrong commit msg, bah!): https://hg.mozilla.org/integration/mozilla-inbound/rev/e6f4331dba0d

Comment 7

6 years ago
https://hg.mozilla.org/mozilla-central/rev/e6f4331dba0d
Assignee: nobody → kyle
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Component: DOM → IPC
QA Contact: general → ipc
You need to log in before you can comment on or make changes to this bug.