Ignore the asynchronous result while stopping keepalive
Currently, onStopped callback are synchronizely triggered when
stop() was called, since the framework don't really care
about the result of stopping keepalive.
However, if keepalive failed to stop for some reason, the
handleStopKeepalive was called mistakenly and trigger additional
callback that fail the test case.
This commit is the behavior change prior to state machine
refactoring, and introduce a stopping state for ignoring the
result in the stopping state.
Bug: 129512753
Bug: 123988249
Test: 1. atest com.android.server.ConnectivityServiceTest \
#testNattSocketKeepalives --generate-new-metrics 100
2. atest FrameworksNetTests
Change-Id: I4fa94e0740ba488fb5fe7ac7c3812c195dd0ec4c
This commit is contained in:
@@ -132,6 +132,7 @@ public class KeepaliveTracker {
|
|||||||
private static final int NOT_STARTED = 1;
|
private static final int NOT_STARTED = 1;
|
||||||
private static final int STARTING = 2;
|
private static final int STARTING = 2;
|
||||||
private static final int STARTED = 3;
|
private static final int STARTED = 3;
|
||||||
|
private static final int STOPPING = 4;
|
||||||
private int mStartedState = NOT_STARTED;
|
private int mStartedState = NOT_STARTED;
|
||||||
|
|
||||||
KeepaliveInfo(@NonNull ISocketKeepaliveCallback callback,
|
KeepaliveInfo(@NonNull ISocketKeepaliveCallback callback,
|
||||||
@@ -314,6 +315,7 @@ public class KeepaliveTracker {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (NOT_STARTED != mStartedState) {
|
if (NOT_STARTED != mStartedState) {
|
||||||
|
mStartedState = STOPPING;
|
||||||
Log.d(TAG, "Stopping keepalive " + mSlot + " on " + mNai.name());
|
Log.d(TAG, "Stopping keepalive " + mSlot + " on " + mNai.name());
|
||||||
if (mType == TYPE_NATT) {
|
if (mType == TYPE_NATT) {
|
||||||
mNai.asyncChannel.sendMessage(CMD_STOP_SOCKET_KEEPALIVE, mSlot);
|
mNai.asyncChannel.sendMessage(CMD_STOP_SOCKET_KEEPALIVE, mSlot);
|
||||||
@@ -456,8 +458,8 @@ public class KeepaliveTracker {
|
|||||||
ki = mKeepalives.get(nai).get(slot);
|
ki = mKeepalives.get(nai).get(slot);
|
||||||
} catch(NullPointerException e) {}
|
} catch(NullPointerException e) {}
|
||||||
if (ki == null) {
|
if (ki == null) {
|
||||||
Log.e(TAG, "Event " + message.what + " for unknown keepalive " + slot + " on "
|
Log.e(TAG, "Event " + message.what + "," + slot + "," + reason
|
||||||
+ nai.name());
|
+ " for unknown keepalive " + slot + " on " + nai.name());
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -476,7 +478,8 @@ public class KeepaliveTracker {
|
|||||||
// messages in order.
|
// messages in order.
|
||||||
// TODO : clarify this code and get rid of mStartedState. Using a StateMachine is an
|
// TODO : clarify this code and get rid of mStartedState. Using a StateMachine is an
|
||||||
// option.
|
// option.
|
||||||
if (reason == SUCCESS && KeepaliveInfo.STARTING == ki.mStartedState) {
|
if (KeepaliveInfo.STARTING == ki.mStartedState) {
|
||||||
|
if (SUCCESS == reason) {
|
||||||
// Keepalive successfully started.
|
// Keepalive successfully started.
|
||||||
if (DBG) Log.d(TAG, "Started keepalive " + slot + " on " + nai.name());
|
if (DBG) Log.d(TAG, "Started keepalive " + slot + " on " + nai.name());
|
||||||
ki.mStartedState = KeepaliveInfo.STARTED;
|
ki.mStartedState = KeepaliveInfo.STARTED;
|
||||||
@@ -486,17 +489,19 @@ public class KeepaliveTracker {
|
|||||||
Log.w(TAG, "Discarded onStarted(" + slot + ") callback");
|
Log.w(TAG, "Discarded onStarted(" + slot + ") callback");
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Keepalive successfully stopped, or error.
|
Log.d(TAG, "Failed to start keepalive " + slot + " on " + nai.name()
|
||||||
if (reason == SUCCESS) {
|
+ ": " + reason);
|
||||||
// The message indicated success stopping : don't call handleStopKeepalive.
|
// The message indicated some error trying to start: do call handleStopKeepalive.
|
||||||
if (DBG) Log.d(TAG, "Successfully stopped keepalive " + slot + " on " + nai.name());
|
|
||||||
} else {
|
|
||||||
// The message indicated some error trying to start or during the course of
|
|
||||||
// keepalive : do call handleStopKeepalive.
|
|
||||||
handleStopKeepalive(nai, slot, reason);
|
handleStopKeepalive(nai, slot, reason);
|
||||||
if (DBG) Log.d(TAG, "Keepalive " + slot + " on " + nai.name() + " error " + reason);
|
|
||||||
}
|
}
|
||||||
|
} else if (KeepaliveInfo.STOPPING == ki.mStartedState) {
|
||||||
|
// The message indicated result of stopping : don't call handleStopKeepalive.
|
||||||
|
Log.d(TAG, "Stopped keepalive " + slot + " on " + nai.name()
|
||||||
|
+ " stopped: " + reason);
|
||||||
ki.mStartedState = KeepaliveInfo.NOT_STARTED;
|
ki.mStartedState = KeepaliveInfo.NOT_STARTED;
|
||||||
|
} else {
|
||||||
|
Log.wtf(TAG, "Event " + message.what + "," + slot + "," + reason
|
||||||
|
+ " for keepalive in wrong state: " + ki.toString());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user