From efdb8454275954e54bd11a59d450cbac30c13da4 Mon Sep 17 00:00:00 2001 From: Rebecca Silberstein Date: Thu, 21 Apr 2016 12:14:41 -0700 Subject: [PATCH 1/2] NativeDaemonConnector: add waitForCallbacks method Add a method that allows callers to wait until all unsolicited responses received from the native daemon during a command are processed. When commands are issued to a native daemon (such as netd) through the NativeDaemonConnector we block until the command response is received. Any responses or events that are a side-effect (considered "unsolicited") of the command are placed in a Message and handled as callbacks. The order of their processing is not guaranteed and, as we have seen from bugreports, can be handled several seconds later - causing the SoftAP that was just set up to be torn down because a late interface down/removed is indistinguishable from a new interface down/removed. This CL adds a method that first checks to make sure callback thread is not the same thread as used for the blocking call. The new waitForCallbacks method uses a CountDownLatch to force the calling thread to wait until all unsolicited responses received from the native daemon during the execution of the command are handled. The wifiFirmwareReload method is also updated to use the new waitForCallbacks method. BUG: 27857665 Change-Id: I3e22978f720b1cbf57fbb64ad4fea73f8c2d408a --- .../android/server/NativeDaemonConnector.java | 25 +++++++++++++++++++ .../server/NetworkManagementService.java | 5 ++++ 2 files changed, 30 insertions(+) diff --git a/services/core/java/com/android/server/NativeDaemonConnector.java b/services/core/java/com/android/server/NativeDaemonConnector.java index 6009984842f46..f5f7732141242 100644 --- a/services/core/java/com/android/server/NativeDaemonConnector.java +++ b/services/core/java/com/android/server/NativeDaemonConnector.java @@ -41,6 +41,7 @@ import java.util.ArrayList; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.LinkedList; @@ -343,6 +344,30 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo rawBuilder.append('\0'); } + /** + * Method that waits until all asychronous notifications sent by the native daemon have + * been processed. This method must not be called on the notification thread or an + * exception will be thrown. + */ + public void waitForCallbacks() { + if (Thread.currentThread() == mLooper.getThread()) { + throw new IllegalStateException("Must not call this method on callback thread"); + } + + final CountDownLatch latch = new CountDownLatch(1); + mCallbackHandler.post(new Runnable() { + @Override + public void run() { + latch.countDown(); + } + }); + try { + latch.await(); + } catch (InterruptedException e) { + Slog.wtf(TAG, "Interrupted while waiting for unsolicited response handling", e); + } + } + /** * Issue the given command to the native daemon and return a single expected * response. diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java index fffd850ac1cba..7458898475797 100644 --- a/services/core/java/com/android/server/NetworkManagementService.java +++ b/services/core/java/com/android/server/NetworkManagementService.java @@ -1507,6 +1507,11 @@ public class NetworkManagementService extends INetworkManagementService.Stub } catch (NativeDaemonConnectorException e) { throw e.rethrowAsParcelableException(); } + + // Ensure that before we return from this command, any asynchronous + // notifications generated before the command completed have been + // processed by all NetworkManagementEventObservers. + mConnector.waitForCallbacks(); } @Override From 636c07d9a9a6e48197706bd9724e5d5db4e2fe9d Mon Sep 17 00:00:00 2001 From: Rebecca Silberstein Date: Thu, 21 Apr 2016 15:50:34 -0700 Subject: [PATCH 2/2] Tethering: interface updates can be handled again When reloading wifi firmware, unsolicited responses from netd were processed after softap had started and caused wifi tethering to be torn down. The NetworkManagementServer.wifiFirmwareReload call has been changed to not only block for the command to finish, but also until all unsolicited messages (interface updates) have been handled. We should now be able to handle interface updates in tethering without suffering from the softap bringup/interface down notification race condition. BUG: 27857665 Change-Id: Ie57cb8f760781b3227df575b577b33667070d63e --- .../core/java/com/android/server/connectivity/Tethering.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java index 79b5978998c4c..c2022d5008d8f 100644 --- a/services/core/java/com/android/server/connectivity/Tethering.java +++ b/services/core/java/com/android/server/connectivity/Tethering.java @@ -273,9 +273,6 @@ public class Tethering extends BaseNetworkObserver { // ignore usb0 down after enabling RNDIS // we will handle disconnect in interfaceRemoved instead if (VDBG) Log.d(TAG, "ignore interface down for " + iface); - } else if (isWifi(iface)) { - // handle disconnect in interfaceRemoved - if (VDBG) Log.d(TAG, "ignore interface down for " + iface); } else if (sm != null) { sm.sendMessage(TetherInterfaceSM.CMD_INTERFACE_DOWN); mIfaces.remove(iface);