From ca80b462be3ddd34c0fa206f9d333c3c533705c0 Mon Sep 17 00:00:00 2001 From: xshu Date: Tue, 10 Jul 2018 17:40:59 -0700 Subject: [LastMileLogger] Drop erroneous timeouts When a new connection is starting, drop any pending timeout messages. When a connection success or failure happen, also drop any pending timeout messages. Bug: 111060673 Test: compile, unit tests Manual test: Enter wrong password for authentication failure Run dumpsys wifi to see the "Last failed connection fates" printout Connect to GoogleGuest successfully Run dumpsys wifi to see both "Last failed connection fates" and "latest fates" printouts and note they are different. Wait 2 minutes to make sure the timeout message is processed Run dumpsys wifi and observe the "Last failed connection fates" log is not over written by the latest packet fates data. Change-Id: Ib1157c5975b667a75714daa6e4963f3ddd888142 --- .../com/android/server/wifi/BaseWifiDiagnostics.java | 2 +- .../java/com/android/server/wifi/ClientModeImpl.java | 8 ++++++-- .../java/com/android/server/wifi/LastMileLogger.java | 17 ++++------------- .../java/com/android/server/wifi/WifiDiagnostics.java | 4 ++-- 4 files changed, 13 insertions(+), 18 deletions(-) (limited to 'service') diff --git a/service/java/com/android/server/wifi/BaseWifiDiagnostics.java b/service/java/com/android/server/wifi/BaseWifiDiagnostics.java index 85fdb6b0d..22eb2e6d0 100644 --- a/service/java/com/android/server/wifi/BaseWifiDiagnostics.java +++ b/service/java/com/android/server/wifi/BaseWifiDiagnostics.java @@ -40,7 +40,7 @@ public class BaseWifiDiagnostics { * @param connectionId A strictly increasing, non-negative, connection identifier * @param event The type of connection event (see CONNECTION_EVENT_* constants) */ - synchronized void reportConnectionEvent(long connectionId, byte event) {} + public synchronized void reportConnectionEvent(long connectionId, byte event) {} public synchronized void captureBugReportData(int reason) { } diff --git a/service/java/com/android/server/wifi/ClientModeImpl.java b/service/java/com/android/server/wifi/ClientModeImpl.java index 3742079f2..046c9c893 100644 --- a/service/java/com/android/server/wifi/ClientModeImpl.java +++ b/service/java/com/android/server/wifi/ClientModeImpl.java @@ -622,7 +622,7 @@ public class ClientModeImpl extends StateMachine { static final int CMD_READ_PACKET_FILTER = BASE + 208; /* Indicates that diagnostics should time out a connection start event. */ - private static final int CMD_DIAGS_CONNECT_TIMEOUT = BASE + 252; + static final int CMD_DIAGS_CONNECT_TIMEOUT = BASE + 252; // Start subscription provisioning with a given provider private static final int CMD_START_SUBSCRIPTION_PROVISIONING = BASE + 254; @@ -2900,7 +2900,8 @@ public class ClientModeImpl extends StateMachine { mInterfaceName, WifiNative.BLUETOOTH_COEXISTENCE_MODE_SENSE); } - private static final long DIAGS_CONNECT_TIMEOUT_MILLIS = 60 * 1000; + @VisibleForTesting + public static final long DIAGS_CONNECT_TIMEOUT_MILLIS = 60 * 1000; private long mDiagsConnectionStartMillis = -1; /** * Inform other components that a new connection attempt is starting. @@ -2914,6 +2915,7 @@ public class ClientModeImpl extends StateMachine { mWrongPasswordNotifier.onNewConnectionAttempt(); // TODO(b/35329124): Remove CMD_DIAGS_CONNECT_TIMEOUT, once ClientModeImpl // grows a proper CONNECTING state. + removeMessages(CMD_DIAGS_CONNECT_TIMEOUT); sendMessageDelayed(CMD_DIAGS_CONNECT_TIMEOUT, mDiagsConnectionStartMillis, DIAGS_CONNECT_TIMEOUT_MILLIS); } @@ -2936,6 +2938,7 @@ public class ClientModeImpl extends StateMachine { // complete. // // TODO(b/34181219): Fix the above. + removeMessages(CMD_DIAGS_CONNECT_TIMEOUT); mWifiDiagnostics.reportConnectionEvent( mDiagsConnectionStartMillis, WifiDiagnostics.CONNECTION_EVENT_SUCCEEDED); break; @@ -2945,6 +2948,7 @@ public class ClientModeImpl extends StateMachine { // where we failed to initiate a connection attempt with supplicant. break; default: + removeMessages(CMD_DIAGS_CONNECT_TIMEOUT); mWifiDiagnostics.reportConnectionEvent( mDiagsConnectionStartMillis, WifiDiagnostics.CONNECTION_EVENT_FAILED); } diff --git a/service/java/com/android/server/wifi/LastMileLogger.java b/service/java/com/android/server/wifi/LastMileLogger.java index 35ce73677..12277f412 100644 --- a/service/java/com/android/server/wifi/LastMileLogger.java +++ b/service/java/com/android/server/wifi/LastMileLogger.java @@ -69,19 +69,10 @@ public class LastMileLogger { mLastMileLogForLastFailure = readTrace(); return; case BaseWifiDiagnostics.CONNECTION_EVENT_TIMEOUT: - if (connectionId >= mPendingConnectionId) { - mPendingConnectionId = -1; - disableTracing(); - return; - } else { - // Spurious failure message. Here's one scenario where this might happen: - // t=00sec start first connection attempt - // t=30sec start second connection attempt - // t=60sec timeout first connection attempt - // We should not stop tracing in this case, since the second connection attempt - // is still in progress. - return; - } + mPendingConnectionId = -1; + disableTracing(); + mLastMileLogForLastFailure = readTrace(); + return; } } diff --git a/service/java/com/android/server/wifi/WifiDiagnostics.java b/service/java/com/android/server/wifi/WifiDiagnostics.java index 595287f3d..224c7140e 100644 --- a/service/java/com/android/server/wifi/WifiDiagnostics.java +++ b/service/java/com/android/server/wifi/WifiDiagnostics.java @@ -203,9 +203,9 @@ class WifiDiagnostics extends BaseWifiDiagnostics { } @Override - synchronized void reportConnectionEvent(long connectionId, byte event) { + public synchronized void reportConnectionEvent(long connectionId, byte event) { mLastMileLogger.reportConnectionEvent(connectionId, event); - if (event == CONNECTION_EVENT_FAILED) { + if (event == CONNECTION_EVENT_FAILED || event == CONNECTION_EVENT_TIMEOUT) { mPacketFatesForLastFailure = fetchPacketFates(); } } -- cgit v1.2.3