Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 16dc6dbf authored by Wink Saville's avatar Wink Saville
Browse files

Fix a loss of data.

Fix a bug in DataConnection state machine where the notification
of disconnection completion was sent before we actually transitioned
to the in active state. Also, change conn.reset to send a response
so the user can know when the transition to the in active state
completes for that also.

bug: 2471897
Change-Id: I5776324ac89a607925d07f4a600bc5b34c3f3ed6
parent aaf15ecb
Loading
Loading
Loading
Loading
+106 −11
Original line number Diff line number Diff line
@@ -51,7 +51,7 @@ import android.util.EventLog;
 *
 * DataConnection {
 *   + mDefaultState {
 *        EVENT_RESET { clearSettings, >mInactiveState }.
 *        EVENT_RESET { clearSettings, notifiyDisconnectCompleted, >mInactiveState }.
 *        EVENT_CONNECT {  notifyConnectCompleted(FailCause.UNKNOWN) }.
 *        EVENT_DISCONNECT { notifyDisconnectCompleted }.
 *
@@ -60,8 +60,10 @@ import android.util.EventLog;
 *        EVENT_GET_LAST_FAIL_DONE,
 *        EVENT_DEACTIVATE_DONE.
 *     }
 *   ++ # mInactiveState {
 *            EVENT_RESET.
 *   ++ # mInactiveState 
 *        e(doNotifications)
 *        x(clearNotifications) {
 *            EVENT_RESET { notifiyDisconnectCompleted }.
 *            EVENT_CONNECT {startConnecting, >mActivatingState }.
 *        }
 *   ++   mActivatingState {
@@ -338,6 +340,8 @@ public abstract class DataConnection extends HierarchicalStateMachine {
        if (DBG) log("NotifyDisconnectCompleted");

        Message msg = dp.onCompletedMsg;
        log(String.format("msg.what=%d msg.obj=%s",
                msg.what, ((msg.obj instanceof String) ? (String) msg.obj : "<no-reason>")));
        AsyncResult.forMessage(msg);
        msg.sendToTarget();

@@ -437,6 +441,9 @@ public abstract class DataConnection extends HierarchicalStateMachine {
                case EVENT_RESET:
                    if (DBG) log("DcDefaultState: msg.what=EVENT_RESET");
                    clearSettings();
                    if (msg.obj != null) {
                        notifyDisconnectCompleted((DisconnectParams) msg.obj);
                    }
                    transitionTo(mInactiveState);
                    break;

@@ -467,9 +474,48 @@ public abstract class DataConnection extends HierarchicalStateMachine {
     * The state machine is inactive and expects a EVENT_CONNECT.
     */
    private class DcInactiveState extends HierarchicalState {
        private ConnectionParams mConnectionParams = null;
        private FailCause mFailCause = null;
        private DisconnectParams mDisconnectParams = null;

        public void setEnterNotificationParams(ConnectionParams cp, FailCause cause) {
            log("DcInactiveState: setEnterNoticationParams cp,cause");
            mConnectionParams = cp;
            mFailCause = cause;
        }

        public void setEnterNotificationParams(DisconnectParams dp) {
          log("DcInactiveState: setEnterNoticationParams dp");
            mDisconnectParams = dp;
        }

        @Override protected void enter() {
            mTag += 1;

            /**
             * Now that we've transitioned to Inactive state we
             * can send notifications. Previously we sent the
             * notifications in the processMessage handler but
             * that caused a race condition because the synchronous
             * call to isInactive.
             */
            if ((mConnectionParams != null) && (mFailCause != null)) {
                log("DcInactiveState: enter notifyConnectCompleted");
                notifyConnectCompleted(mConnectionParams, mFailCause);
            }
            if (mDisconnectParams != null) {
              log("DcInactiveState: enter notifyDisconnectCompleted");
                notifyDisconnectCompleted(mDisconnectParams);
            }
        }

        @Override protected void exit() {
            // clear notifications
            mConnectionParams = null;
            mFailCause = null;
            mDisconnectParams = null;
        }

        @Override protected boolean processMessage(Message msg) {
            boolean retVal;

@@ -478,6 +524,9 @@ public abstract class DataConnection extends HierarchicalStateMachine {
                    if (DBG) {
                        log("DcInactiveState: msg.what=EVENT_RESET, ignore we're already reset");
                    }
                    if (msg.obj != null) {
                        notifyDisconnectCompleted((DisconnectParams) msg.obj);
                    }
                    retVal = true;
                    break;

@@ -526,12 +575,14 @@ public abstract class DataConnection extends HierarchicalStateMachine {
                    switch (result) {
                        case SUCCESS:
                            // All is well
                            notifyConnectCompleted(cp, FailCause.NONE);
                            mActiveState.setEnterNotificationParams(cp, FailCause.NONE);
                            transitionTo(mActiveState);
                            break;
                        case ERR_BadCommand:
                            // Vendor ril rejected the command and didn't connect.
                            notifyConnectCompleted(cp, result.mFailCause);
                            // Transition to inactive but send notifications after
                            // we've entered the mInactive state.
                            mInactiveState.setEnterNotificationParams(cp, result.mFailCause);
                            transitionTo(mInactiveState);
                            break;
                        case ERR_BadDns:
@@ -565,7 +616,9 @@ public abstract class DataConnection extends HierarchicalStateMachine {
                            int rilFailCause = ((int[]) (ar.result))[0];
                            cause = getFailCauseFromRequest(rilFailCause);
                        }
                         notifyConnectCompleted(cp, cause);
                        // Transition to inactive but send notifications after
                        // we've entered the mInactive state.
                         mInactiveState.setEnterNotificationParams(cp, cause);
                         transitionTo(mInactiveState);
                    } else {
                        if (DBG) {
@@ -591,6 +644,35 @@ public abstract class DataConnection extends HierarchicalStateMachine {
     * The state machine is connected, expecting an EVENT_DISCONNECT.
     */
    private class DcActiveState extends HierarchicalState {
        private ConnectionParams mConnectionParams = null;
        private FailCause mFailCause = null;

        public void setEnterNotificationParams(ConnectionParams cp, FailCause cause) {
            log("DcInactiveState: setEnterNoticationParams cp,cause");
            mConnectionParams = cp;
            mFailCause = cause;
        }

        @Override public void enter() {
            /**
             * Now that we've transitioned to Active state we
             * can send notifications. Previously we sent the
             * notifications in the processMessage handler but
             * that caused a race condition because the synchronous
             * call to isActive.
             */
            if ((mConnectionParams != null) && (mFailCause != null)) {
                log("DcActiveState: enter notifyConnectCompleted");
                notifyConnectCompleted(mConnectionParams, mFailCause);
            }
        }

        @Override protected void exit() {
            // clear notifications
            mConnectionParams = null;
            mFailCause = null;
        }

        @Override protected boolean processMessage(Message msg) {
            boolean retVal;

@@ -627,7 +709,9 @@ public abstract class DataConnection extends HierarchicalStateMachine {
                    AsyncResult ar = (AsyncResult) msg.obj;
                    DisconnectParams dp = (DisconnectParams) ar.userObj;
                    if (dp.tag == mTag) {
                        notifyDisconnectCompleted((DisconnectParams) ar.userObj);
                        // Transition to inactive but send notifications after
                        // we've entered the mInactive state.
                        mInactiveState.setEnterNotificationParams((DisconnectParams) ar.userObj);
                        transitionTo(mInactiveState);
                    } else {
                        if (DBG) log("DcDisconnectState EVENT_DEACTIVATE_DONE stale dp.tag="
@@ -660,7 +744,9 @@ public abstract class DataConnection extends HierarchicalStateMachine {
                    ConnectionParams cp = (ConnectionParams) ar.userObj;
                    if (cp.tag == mTag) {
                        if (DBG) log("DcDisconnectingBadDnsState msg.what=EVENT_DEACTIVATE_DONE");
                        notifyConnectCompleted(cp, FailCause.UNKNOWN);
                        // Transition to inactive but send notifications after
                        // we've entered the mInactive state.
                        mInactiveState.setEnterNotificationParams(cp, FailCause.UNKNOWN);
                        transitionTo(mInactiveState);
                    } else {
                        if (DBG) log("DcDisconnectingBadDnsState EVENT_DEACTIVE_DONE stale dp.tag="
@@ -683,9 +769,12 @@ public abstract class DataConnection extends HierarchicalStateMachine {

    /**
     * Disconnect from the network.
     *
     * @param onCompletedMsg is sent with its msg.obj as an AsyncResult object.
     *        With AsyncResult.userObj set to the original msg.obj.
     */
    public void reset() {
        sendMessage(obtainMessage(EVENT_RESET));
    public void reset(Message onCompletedMsg) {
        sendMessage(obtainMessage(EVENT_RESET, new DisconnectParams(onCompletedMsg)));
    }

    /**
@@ -726,6 +815,9 @@ public abstract class DataConnection extends HierarchicalStateMachine {
    // ****** The following are used for debugging.

    /**
     * TODO: This should be an asynchronous call and we wouldn't
     * have to use handle the notification in the DcInactiveState.enter.
     *
     * @return true if the state machine is in the inactive state.
     */
    public boolean isInactive() {
@@ -734,7 +826,10 @@ public abstract class DataConnection extends HierarchicalStateMachine {
    }

    /**
     * @return true if the state machine is in the inactive state.
     * TODO: This should be an asynchronous call and we wouldn't
     * have to use handle the notification in the DcActiveState.enter.
     *
     * @return true if the state machine is in the active state.
     */
    public boolean isActive() {
        boolean retVal = getCurrentState() == mActiveState;
+6 −0
Original line number Diff line number Diff line
@@ -101,6 +101,7 @@ public abstract class DataConnectionTracker extends Handler {
    protected static final int EVENT_CDMA_OTA_PROVISION = 35;
    protected static final int EVENT_RESTART_RADIO = 36;
    protected static final int EVENT_SET_MASTER_DATA_ENABLE = 37;
    protected static final int EVENT_RESET_DONE = 38;

    /***** Constants *****/

@@ -265,6 +266,7 @@ public abstract class DataConnectionTracker extends Handler {
    protected abstract void onRadioOffOrNotAvailable();
    protected abstract void onDataSetupComplete(AsyncResult ar);
    protected abstract void onDisconnectDone(AsyncResult ar);
    protected abstract void onResetDone(AsyncResult ar);
    protected abstract void onVoiceCallStarted();
    protected abstract void onVoiceCallEnded();
    protected abstract void onCleanUpConnection(boolean tearDown, String reason);
@@ -331,6 +333,10 @@ public abstract class DataConnectionTracker extends Handler {
                onSetDataEnabled(enabled);
                break;

            case EVENT_RESET_DONE:
                onResetDone((AsyncResult) msg.obj);
                break;

            default:
                Log.e("DATA", "Unidentified event = " + msg.what);
                break;
+33 −12
Original line number Diff line number Diff line
@@ -368,7 +368,7 @@ public final class CdmaDataConnectionTracker extends DataConnectionTracker {
     * @param reason reason for the clean up.
     */
    private void cleanUpConnection(boolean tearDown, String reason) {
        if (DBG) log("Clean up connection due to " + reason);
        if (DBG) log("cleanUpConnection: reason: " + reason);

        // Clear the reconnect alarm, if set.
        if (mReconnectIntent != null) {
@@ -380,25 +380,25 @@ public final class CdmaDataConnectionTracker extends DataConnectionTracker {

        setState(State.DISCONNECTING);

        for (DataConnection connBase : dataConnectionList) {
            CdmaDataConnection conn = (CdmaDataConnection) connBase;

        boolean notificationDeferred = false;
        for (DataConnection conn : dataConnectionList) {
            if(conn != null) {
                if (tearDown) {
                    Message msg = obtainMessage(EVENT_DISCONNECT_DONE, reason);
                    conn.disconnect(msg);
                    if (DBG) log("cleanUpConnection: teardown, call conn.disconnect");
                    conn.disconnect(obtainMessage(EVENT_DISCONNECT_DONE, reason));
                } else {
                    conn.reset();
                    if (DBG) log("cleanUpConnection: !tearDown, call conn.reset");
                    conn.reset(obtainMessage(EVENT_RESET_DONE, reason));
                }
                notificationDeferred = true;
            }
        }

        stopNetStatPoll();

        if (!tearDown) {
            setState(State.IDLE);
            phone.notifyDataConnection(reason);
            mIsApnActive = false;
        if (!notificationDeferred) {
            if (DBG) log("cleanupConnection: !tearDown && !resettingConn");
            gotoIdleAndNotifyDataConnection(reason);
        }
    }

@@ -622,6 +622,13 @@ public final class CdmaDataConnectionTracker extends DataConnectionTracker {
        setState(State.FAILED);
    }

    private void gotoIdleAndNotifyDataConnection(String reason) {
        if (DBG) log("gotoIdleAndNotifyDataConnection: reason=" + reason);
        setState(State.IDLE);
        phone.notifyDataConnection(reason);
        mIsApnActive = false;
    }

    protected void onRecordsLoaded() {
        if (state == State.FAILED) {
            cleanUpConnection(false, null);
@@ -731,7 +738,7 @@ public final class CdmaDataConnectionTracker extends DataConnectionTracker {
    }

    /**
     * @override com.android.internal.telephony.DataConnectionTracker
     * Called when EVENT_DISCONNECT_DONE is received.
     */
    protected void onDisconnectDone(AsyncResult ar) {
        if(DBG) log("EVENT_DISCONNECT_DONE");
@@ -761,6 +768,20 @@ public final class CdmaDataConnectionTracker extends DataConnectionTracker {
      }
    }

    /**
     * Called when EVENT_RESET_DONE is received so goto
     * IDLE state and send notifications to those interested.
     */
    @Override
    protected void onResetDone(AsyncResult ar) {
      if (DBG) log("EVENT_RESET_DONE");
      String reason = null;
      if (ar.userObj instanceof String) {
          reason = (String) ar.userObj;
      }
      gotoIdleAndNotifyDataConnection(reason);
    }

    /**
     * @override com.android.internal.telephony.DataConnectionTracker
     */
+32 −8
Original line number Diff line number Diff line
@@ -502,20 +502,22 @@ public final class GsmDataConnectionTracker extends DataConnectionTracker {

        setState(State.DISCONNECTING);

        boolean notificationDeferred = false;
        for (DataConnection conn : pdpList) {
            if (tearDown) {
                Message msg = obtainMessage(EVENT_DISCONNECT_DONE, reason);
                conn.disconnect(msg);
                if (DBG) log("cleanUpConnection: teardown, call conn.disconnect");
                conn.disconnect(obtainMessage(EVENT_DISCONNECT_DONE, reason));
            } else {
                conn.reset();
                if (DBG) log("cleanUpConnection: !tearDown, call conn.reset");
                conn.reset(obtainMessage(EVENT_RESET_DONE, reason));
            }
            notificationDeferred = true;
        }
        stopNetStatPoll();

        if (!tearDown) {
            setState(State.IDLE);
            phone.notifyDataConnection(reason);
            mActiveApn = null;
        if (!notificationDeferred) {
            if (DBG) log("cleanupConnection: !tearDown && !resettingConn");
            gotoIdleAndNotifyDataConnection(reason);
        }
    }

@@ -749,6 +751,13 @@ public final class GsmDataConnectionTracker extends DataConnectionTracker {
        mReregisterOnReconnectFailure = false;
    }

    private void gotoIdleAndNotifyDataConnection(String reason) {
        if (DBG) log("gotoIdleAndNotifyDataConnection: reason=" + reason);
        setState(State.IDLE);
        phone.notifyDataConnection(reason);
        mActiveApn = null;
    }

    /**
     * This is a kludge to deal with the fact that
     * the PDP state change notification doesn't always work
@@ -1172,6 +1181,9 @@ public final class GsmDataConnectionTracker extends DataConnectionTracker {
        }
    }

    /**
     * Called when EVENT_DISCONNECT_DONE is received.
     */
    protected void onDisconnectDone(AsyncResult ar) {
        String reason = null;
        if(DBG) log("EVENT_DISCONNECT_DONE");
@@ -1186,6 +1198,19 @@ public final class GsmDataConnectionTracker extends DataConnectionTracker {
        }
    }

    /**
     * Called when EVENT_RESET_DONE is received.
     */
    @Override
    protected void onResetDone(AsyncResult ar) {
        if (DBG) log("EVENT_RESET_DONE");
        String reason = null;
        if (ar.userObj instanceof String) {
            reason = (String) ar.userObj;
        }
        gotoIdleAndNotifyDataConnection(reason);
    }

    protected void onPollPdp() {
        if (state == State.CONNECTED) {
            // only poll when connected
@@ -1487,5 +1512,4 @@ public final class GsmDataConnectionTracker extends DataConnectionTracker {
    protected void log(String s) {
        Log.d(LOG_TAG, "[GsmDataConnectionTracker] " + s);
    }

}