From 8a5bc9513ceb608391721198df47c1acee0af2cd Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Fri, 21 Jun 2019 09:37:39 -0700 Subject: SupplicantHal/HostapdHal: Wait for death When wpa_supplicant/hostapd is being terminated, ensure that we block for a small timeout (50 ms) to ensure that the daemon is indeed dead. This prevents a potential race when wifi/tethering is quickly toggled & the previous instance of the daemon is not fully terminated before the next toggle on. Bug: 134729435 Test: Manually toggled wifi/tethering multiple times in quick succession. Test: Will send for full regression testing. Change-Id: I0cd14e22bba21a4f7fdebd8e68642bcae9739b25 --- .../java/com/android/server/wifi/HostapdHal.java | 31 ++++++++++++++++++--- .../android/server/wifi/SupplicantStaIfaceHal.java | 32 +++++++++++++++++++--- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/service/java/com/android/server/wifi/HostapdHal.java b/service/java/com/android/server/wifi/HostapdHal.java index 69c787f48..ee96fb269 100644 --- a/service/java/com/android/server/wifi/HostapdHal.java +++ b/service/java/com/android/server/wifi/HostapdHal.java @@ -39,6 +39,9 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.NoSuchElementException; +import java.util.Random; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import javax.annotation.concurrent.ThreadSafe; @@ -51,6 +54,8 @@ public class HostapdHal { private static final String TAG = "HostapdHal"; @VisibleForTesting public static final String HAL_INSTANCE_NAME = "default"; + @VisibleForTesting + public static final long WAIT_FOR_DEATH_TIMEOUT_MS = 50L; private final Object mLock = new Object(); private boolean mVerboseLoggingEnabled = false; @@ -230,11 +235,11 @@ public class HostapdHal { * Link to death for IHostapd object. * @return true on success, false otherwise. */ - private boolean linkToHostapdDeath() { + private boolean linkToHostapdDeath(HwRemoteBinder.DeathRecipient deathRecipient, long cookie) { synchronized (mLock) { if (mIHostapd == null) return false; try { - if (!mIHostapd.linkToDeath(mHostapdDeathRecipient, ++mDeathRecipientCookie)) { + if (!mIHostapd.linkToDeath(deathRecipient, cookie)) { Log.wtf(TAG, "Error on linkToDeath on IHostapd"); hostapdServiceDiedHandler(mDeathRecipientCookie); return false; @@ -282,7 +287,7 @@ public class HostapdHal { Log.e(TAG, "Got null IHostapd service. Stopping hostapd HIDL startup"); return false; } - if (!linkToHostapdDeath()) { + if (!linkToHostapdDeath(mHostapdDeathRecipient, ++mDeathRecipientCookie)) { mIHostapd = null; return false; } @@ -486,10 +491,19 @@ public class HostapdHal { } /** - * Terminate the hostapd daemon. + * Terminate the hostapd daemon & wait for it's death. */ public void terminate() { synchronized (mLock) { + // Register for a new death listener to block until hostapd is dead. + final long waitForDeathCookie = new Random().nextLong(); + final CountDownLatch waitForDeathLatch = new CountDownLatch(1); + linkToHostapdDeath((cookie) -> { + Log.d(TAG, "IHostapd died: cookie=" + cookie); + if (cookie != waitForDeathCookie) return; + waitForDeathLatch.countDown(); + }, waitForDeathCookie); + final String methodStr = "terminate"; if (!checkHostapdAndLogFailure(methodStr)) return; try { @@ -497,6 +511,15 @@ public class HostapdHal { } catch (RemoteException e) { handleRemoteException(e, methodStr); } + + // Now wait for death listener callback to confirm that it's dead. + try { + if (!waitForDeathLatch.await(WAIT_FOR_DEATH_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + Log.w(TAG, "Timed out waiting for confirmation of hostapd death"); + } + } catch (InterruptedException e) { + Log.w(TAG, "Failed to wait for hostapd death"); + } } } diff --git a/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java b/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java index bed66d926..86518c761 100644 --- a/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java +++ b/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java @@ -86,6 +86,9 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; +import java.util.Random; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -108,6 +111,8 @@ public class SupplicantStaIfaceHal { public static final String INIT_STOP_PROPERTY = "ctl.stop"; @VisibleForTesting public static final String INIT_SERVICE_NAME = "wpa_supplicant"; + @VisibleForTesting + public static final long WAIT_FOR_DEATH_TIMEOUT_MS = 50L; /** * Regex pattern for extracting the wps device type bytes. * Matches a strings like the following: "--"; @@ -262,11 +267,12 @@ public class SupplicantStaIfaceHal { } } - private boolean linkToSupplicantDeath() { + private boolean linkToSupplicantDeath( + HwRemoteBinder.DeathRecipient deathRecipient, long cookie) { synchronized (mLock) { if (mISupplicant == null) return false; try { - if (!mISupplicant.linkToDeath(mSupplicantDeathRecipient, ++mDeathRecipientCookie)) { + if (!mISupplicant.linkToDeath(deathRecipient, cookie)) { Log.wtf(TAG, "Error on linkToDeath on ISupplicant"); supplicantServiceDiedHandler(mDeathRecipientCookie); return false; @@ -294,7 +300,7 @@ public class SupplicantStaIfaceHal { Log.e(TAG, "Got null ISupplicant service. Stopping supplicant HIDL startup"); return false; } - if (!linkToSupplicantDeath()) { + if (!linkToSupplicantDeath(mSupplicantDeathRecipient, ++mDeathRecipientCookie)) { return false; } } @@ -645,10 +651,19 @@ public class SupplicantStaIfaceHal { } /** - * Terminate the supplicant daemon. + * Terminate the supplicant daemon & wait for it's death. */ public void terminate() { synchronized (mLock) { + // Register for a new death listener to block until supplicant is dead. + final long waitForDeathCookie = new Random().nextLong(); + final CountDownLatch waitForDeathLatch = new CountDownLatch(1); + linkToSupplicantDeath((cookie) -> { + Log.d(TAG, "ISupplicant died: cookie=" + cookie); + if (cookie != waitForDeathCookie) return; + waitForDeathLatch.countDown(); + }, waitForDeathCookie); + if (isV1_1()) { Log.i(TAG, "Terminating supplicant using HIDL"); terminate_V1_1(); @@ -656,6 +671,15 @@ public class SupplicantStaIfaceHal { Log.i(TAG, "Terminating supplicant using init"); mPropertyService.set(INIT_STOP_PROPERTY, INIT_SERVICE_NAME); } + + // Now wait for death listener callback to confirm that it's dead. + try { + if (!waitForDeathLatch.await(WAIT_FOR_DEATH_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + Log.w(TAG, "Timed out waiting for confirmation of supplicant death"); + } + } catch (InterruptedException e) { + Log.w(TAG, "Failed to wait for supplicant death"); + } } } -- cgit v1.2.3