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
diff --git a/telephony/java/com/android/internal/telephony/DataConnection.java b/telephony/java/com/android/internal/telephony/DataConnection.java
index 562a9f8..521072e 100644
--- a/telephony/java/com/android/internal/telephony/DataConnection.java
+++ b/telephony/java/com/android/internal/telephony/DataConnection.java
@@ -51,7 +51,7 @@
*
* DataConnection {
* + mDefaultState {
- * EVENT_RESET { clearSettings, >mInactiveState }.
+ * EVENT_RESET { clearSettings, notifiyDisconnectCompleted, >mInactiveState }.
* EVENT_CONNECT { notifyConnectCompleted(FailCause.UNKNOWN) }.
* EVENT_DISCONNECT { notifyDisconnectCompleted }.
*
@@ -60,8 +60,10 @@
* 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 @@
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 @@
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 @@
* 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 @@
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 @@
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 @@
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 @@
* 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 @@
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 @@
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 @@
/**
* 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 @@
// ****** 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 @@
}
/**
- * @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;
diff --git a/telephony/java/com/android/internal/telephony/DataConnectionTracker.java b/telephony/java/com/android/internal/telephony/DataConnectionTracker.java
index cab7b81..e8e18a1 100644
--- a/telephony/java/com/android/internal/telephony/DataConnectionTracker.java
+++ b/telephony/java/com/android/internal/telephony/DataConnectionTracker.java
@@ -101,6 +101,7 @@
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 @@
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 @@
onSetDataEnabled(enabled);
break;
+ case EVENT_RESET_DONE:
+ onResetDone((AsyncResult) msg.obj);
+ break;
+
default:
Log.e("DATA", "Unidentified event = " + msg.what);
break;
diff --git a/telephony/java/com/android/internal/telephony/cdma/CdmaDataConnectionTracker.java b/telephony/java/com/android/internal/telephony/cdma/CdmaDataConnectionTracker.java
index b5461bf..9218715 100644
--- a/telephony/java/com/android/internal/telephony/cdma/CdmaDataConnectionTracker.java
+++ b/telephony/java/com/android/internal/telephony/cdma/CdmaDataConnectionTracker.java
@@ -368,7 +368,7 @@
* @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 @@
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 @@
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 @@
}
/**
- * @override com.android.internal.telephony.DataConnectionTracker
+ * Called when EVENT_DISCONNECT_DONE is received.
*/
protected void onDisconnectDone(AsyncResult ar) {
if(DBG) log("EVENT_DISCONNECT_DONE");
@@ -762,6 +769,20 @@
}
/**
+ * 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
*/
protected void onVoiceCallStarted() {
diff --git a/telephony/java/com/android/internal/telephony/gsm/GsmDataConnectionTracker.java b/telephony/java/com/android/internal/telephony/gsm/GsmDataConnectionTracker.java
index 30beaaa..f26e54e 100644
--- a/telephony/java/com/android/internal/telephony/gsm/GsmDataConnectionTracker.java
+++ b/telephony/java/com/android/internal/telephony/gsm/GsmDataConnectionTracker.java
@@ -502,20 +502,22 @@
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 @@
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 @@
}
}
+ /**
+ * 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 @@
}
}
+ /**
+ * 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 @@
protected void log(String s) {
Log.d(LOG_TAG, "[GsmDataConnectionTracker] " + s);
}
-
}
