Last Comment Bug 708926 - RIL does not reconnect on socket connection loss
: RIL does not reconnect on socket connection loss
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: IPC (show other bugs)
: unspecified
: All Gonk (Firefox OS)
: -- normal (vote)
: mozilla11
Assigned To: Kyle Machulis [:kmachulis] [:qdot]
:
Mentors:
Depends on:
Blocks: b2g-ril 711315
  Show dependency treegraph
 
Reported: 2011-12-08 17:51 PST by Kyle Machulis [:kmachulis] [:qdot]
Modified: 2011-12-16 13:39 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Adding reconnection capabilities to RIL IPC thread (3.86 KB, patch)
2011-12-12 12:32 PST, Kyle Machulis [:kmachulis] [:qdot]
no flags Details | Diff | Splinter Review
RIL Patch with comment/return fixes (3.86 KB, patch)
2011-12-12 14:18 PST, Kyle Machulis [:kmachulis] [:qdot]
cjones.bugs: review+
Details | Diff | Splinter Review
Post-Review RIL Patch with nits picked (3.86 KB, patch)
2011-12-14 15:12 PST, Kyle Machulis [:kmachulis] [:qdot]
no flags Details | Diff | Splinter Review

Description Kyle Machulis [:kmachulis] [:qdot] 2011-12-08 17:51:12 PST
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.
Comment 1 Kyle Machulis [:kmachulis] [:qdot] 2011-12-12 12:32:02 PST
Created attachment 581010 [details] [diff] [review]
Adding reconnection capabilities to RIL IPC thread
Comment 2 Andreas Gal :gal 2011-12-12 13:12:54 PST
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.
Comment 3 Kyle Machulis [:kmachulis] [:qdot] 2011-12-12 14:18:49 PST
Created attachment 581050 [details] [diff] [review]
RIL Patch with comment/return fixes
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-14 06:32:01 PST
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.
Comment 5 Kyle Machulis [:kmachulis] [:qdot] 2011-12-14 15:12:38 PST
Created attachment 581809 [details] [diff] [review]
Post-Review RIL Patch with nits picked
Comment 6 Philipp von Weitershausen [:philikon] 2011-12-15 22:19:25 PST
Pushed (with the wrong commit msg, bah!): https://hg.mozilla.org/integration/mozilla-inbound/rev/e6f4331dba0d
Comment 7 Ed Morley [:emorley] 2011-12-16 05:55:01 PST
https://hg.mozilla.org/mozilla-central/rev/e6f4331dba0d

Note You need to log in before you can comment on or make changes to this bug.