From 0529858b9ccf5e91187a9c64df4754f91b5d55f2 Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Sun, 13 Jun 2021 16:52:05 +0000 Subject: [PATCH] Make NsdService only start the native daemon when needed and automatically clean it up. Currently, NsdService starts the native mdnsresponder daemon if any NsdManager connect to it, which results in that when any constant service holds the NsdManager connection, the device would always be in the mdns multicast group whatever the connection is not used or not. This is because mdnsresponder will join the multicast group when it starts. To solve this problem, start the native daemon only when needed, and clean it up after the given idle timeout. 1. Start the native daemon when a new request come. 2. If there is no pending request, clean up the daemon after 3 seconds of idle time. Bug: 181810560 Test: atest NsdManagerTest NsdServiceTest Original-Change: https://android-review.googlesource.com/1717479 Merged-In: I3eb04552f6cf6c0c68c07abffe751bb4d0669215 Change-Id: I3eb04552f6cf6c0c68c07abffe751bb4d0669215 --- core/java/android/net/nsd/NsdManager.java | 3 + .../java/com/android/server/NsdService.java | 82 ++++++++++++++----- 2 files changed, 66 insertions(+), 19 deletions(-) diff --git a/core/java/android/net/nsd/NsdManager.java b/core/java/android/net/nsd/NsdManager.java index 64f20b839a..5a25cfc00c 100644 --- a/core/java/android/net/nsd/NsdManager.java +++ b/core/java/android/net/nsd/NsdManager.java @@ -199,6 +199,9 @@ public final class NsdManager { /** @hide */ public static final int RESOLVE_SERVICE_SUCCEEDED = BASE + 20; + /** @hide */ + public static final int DAEMON_CLEANUP = BASE + 21; + /** @hide */ public static final int ENABLE = BASE + 24; /** @hide */ diff --git a/services/core/java/com/android/server/NsdService.java b/services/core/java/com/android/server/NsdService.java index 84f40cb4b8..a481a6a5d3 100644 --- a/services/core/java/com/android/server/NsdService.java +++ b/services/core/java/com/android/server/NsdService.java @@ -61,6 +61,7 @@ public class NsdService extends INsdManager.Stub { private static final String MDNS_TAG = "mDnsConnector"; private static final boolean DBG = true; + private static final long CLEANUP_DELAY_MS = 3000; private final Context mContext; private final NsdSettings mNsdSettings; @@ -77,6 +78,7 @@ public class NsdService extends INsdManager.Stub { private final SparseArray mIdToClientInfoMap= new SparseArray<>(); private final AsyncChannel mReplyChannel = new AsyncChannel(); + private final long mCleanupDelayMs; private static final int INVALID_ID = 0; private int mUniqueId = 1; @@ -92,6 +94,22 @@ public class NsdService extends INsdManager.Stub { return NsdManager.nameOf(what); } + void maybeStartDaemon() { + mDaemon.maybeStart(); + maybeScheduleStop(); + } + + void maybeScheduleStop() { + if (!isAnyRequestActive()) { + cancelStop(); + sendMessageDelayed(NsdManager.DAEMON_CLEANUP, mCleanupDelayMs); + } + } + + void cancelStop() { + this.removeMessages(NsdManager.DAEMON_CLEANUP); + } + /** * Observes the NSD on/off setting, and takes action when changed. */ @@ -151,10 +169,6 @@ public class NsdService extends INsdManager.Stub { cInfo.expungeAllRequests(); mClients.remove(msg.replyTo); } - //Last client - if (mClients.size() == 0) { - mDaemon.stop(); - } break; case AsyncChannel.CMD_CHANNEL_FULL_CONNECTION: AsyncChannel ac = new AsyncChannel(); @@ -180,6 +194,9 @@ public class NsdService extends INsdManager.Stub { replyToMessage(msg, NsdManager.RESOLVE_SERVICE_FAILED, NsdManager.FAILURE_INTERNAL_ERROR); break; + case NsdManager.DAEMON_CLEANUP: + mDaemon.maybeStop(); + break; case NsdManager.NATIVE_DAEMON_EVENT: default: Slog.e(TAG, "Unhandled " + msg); @@ -212,16 +229,13 @@ public class NsdService extends INsdManager.Stub { @Override public void enter() { sendNsdStateChangeBroadcast(true); - if (mClients.size() > 0) { - mDaemon.start(); - } } @Override public void exit() { - if (mClients.size() > 0) { - mDaemon.stop(); - } + // TODO: it is incorrect to stop the daemon without expunging all requests + // and sending error callbacks to clients. + maybeScheduleStop(); } private boolean requestLimitReached(ClientInfo clientInfo) { @@ -236,12 +250,15 @@ public class NsdService extends INsdManager.Stub { clientInfo.mClientIds.put(clientId, globalId); clientInfo.mClientRequests.put(clientId, what); mIdToClientInfoMap.put(globalId, clientInfo); + // Remove the cleanup event because here comes a new request. + cancelStop(); } private void removeRequestMap(int clientId, int globalId, ClientInfo clientInfo) { clientInfo.mClientIds.delete(clientId); clientInfo.mClientRequests.delete(clientId); mIdToClientInfoMap.remove(globalId); + maybeScheduleStop(); } @Override @@ -251,14 +268,12 @@ public class NsdService extends INsdManager.Stub { int id; switch (msg.what) { case AsyncChannel.CMD_CHANNEL_HALF_CONNECTED: - //First client - if (msg.arg1 == AsyncChannel.STATUS_SUCCESSFUL && - mClients.size() == 0) { - mDaemon.start(); - } return NOT_HANDLED; case AsyncChannel.CMD_CHANNEL_DISCONNECTED: return NOT_HANDLED; + } + + switch (msg.what) { case NsdManager.DISABLE: //TODO: cleanup clients transitionTo(mDisabledState); @@ -274,6 +289,7 @@ public class NsdService extends INsdManager.Stub { break; } + maybeStartDaemon(); id = getUniqueId(); if (discoverServices(id, servInfo.getServiceType())) { if (DBG) { @@ -316,6 +332,7 @@ public class NsdService extends INsdManager.Stub { break; } + maybeStartDaemon(); id = getUniqueId(); if (registerService(id, (NsdServiceInfo) msg.obj)) { if (DBG) Slog.d(TAG, "Register " + msg.arg2 + " " + id); @@ -357,6 +374,7 @@ public class NsdService extends INsdManager.Stub { break; } + maybeStartDaemon(); id = getUniqueId(); if (resolveService(id, servInfo)) { clientInfo.mResolvedService = new NsdServiceInfo(); @@ -513,6 +531,10 @@ public class NsdService extends INsdManager.Stub { } } + private boolean isAnyRequestActive() { + return mIdToClientInfoMap.size() != 0; + } + private String unescape(String s) { StringBuilder sb = new StringBuilder(s.length()); for (int i = 0; i < s.length(); ++i) { @@ -538,7 +560,9 @@ public class NsdService extends INsdManager.Stub { } @VisibleForTesting - NsdService(Context ctx, NsdSettings settings, Handler handler, DaemonConnectionSupplier fn) { + NsdService(Context ctx, NsdSettings settings, Handler handler, + DaemonConnectionSupplier fn, long cleanupDelayMs) { + mCleanupDelayMs = cleanupDelayMs; mContext = ctx; mNsdSettings = settings; mNsdStateMachine = new NsdStateMachine(TAG, handler); @@ -552,7 +576,8 @@ public class NsdService extends INsdManager.Stub { HandlerThread thread = new HandlerThread(TAG); thread.start(); Handler handler = new Handler(thread.getLooper()); - NsdService service = new NsdService(context, settings, handler, DaemonConnection::new); + NsdService service = new NsdService(context, settings, handler, + DaemonConnection::new, CLEANUP_DELAY_MS); service.mDaemonCallback.awaitConnection(); return service; } @@ -681,12 +706,16 @@ public class NsdService extends INsdManager.Stub { @VisibleForTesting public static class DaemonConnection { final NativeDaemonConnector mNativeConnector; + boolean mIsStarted = false; DaemonConnection(NativeCallbackReceiver callback) { mNativeConnector = new NativeDaemonConnector(callback, "mdns", 10, MDNS_TAG, 25, null); new Thread(mNativeConnector, MDNS_TAG).start(); } + /** + * Executes the specified cmd on the daemon. + */ public boolean execute(Object... args) { if (DBG) { Slog.d(TAG, "mdnssd " + Arrays.toString(args)); @@ -700,12 +729,26 @@ public class NsdService extends INsdManager.Stub { return true; } - public void start() { + /** + * Starts the daemon if it is not already started. + */ + public void maybeStart() { + if (mIsStarted) { + return; + } execute("start-service"); + mIsStarted = true; } - public void stop() { + /** + * Stops the daemon if it is started. + */ + public void maybeStop() { + if (!mIsStarted) { + return; + } execute("stop-service"); + mIsStarted = false; } } @@ -864,6 +907,7 @@ public class NsdService extends INsdManager.Stub { } mClientIds.clear(); mClientRequests.clear(); + mNsdStateMachine.maybeScheduleStop(); } // mClientIds is a sparse array of listener id -> mDnsClient id. For a given mDnsClient id,