From 578e0dc6084bb9e382b0426a0879f157292f5db1 Mon Sep 17 00:00:00 2001
From: Remi NGUYEN VAN <reminv@google.com>
Date: Thu, 1 Feb 2024 15:06:08 +0900
Subject: [PATCH] Initialize serviceCache before first use

The comment that "The very first posted task is to initialize the
service cache, so it will be always set in other tasks running on
the handler" did not consider that if the looper was provided by the
caller, it is possible for other tasks to run on it before the
serviceCache is initialized. In particular, if the caller calls
registerListener on the looper thread right after calling the
constructor, this would be executed before the posted initialization
task.

In practice this does not happen as MdnsDiscoveryManager is created in
the NsdService constructor, and any usage of it is on messages posted to
the NsdService handler thread after the constructor has been called; so
the service cache initialization does happen before any other
MdnsDiscoveryManager task.

As this is error-prone, just initialize the service cache before first
use on the handler thread, as it is only used for
createServiceTypeClient, which runs on the handler thread.

Test: atest
Change-Id: Ie341bb71ef327dceb4f0aa6fc2d241d6e9cbb900
---
 .../connectivity/mdns/MdnsDiscoveryManager.java    | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java b/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java
index 508807ee9f..1d6039c05a 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java
@@ -55,7 +55,8 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback {
     @NonNull private final DiscoveryExecutor discoveryExecutor;
     @NonNull private final MdnsFeatureFlags mdnsFeatureFlags;
 
-    // Only accessed on the handler thread, set to non-null right after initializing the handler
+    // Only accessed on the handler thread, initialized before first use
+    @Nullable
     private MdnsServiceCache serviceCache;
 
     private static class PerSocketServiceTypeClients {
@@ -129,11 +130,6 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback {
         this.perSocketServiceTypeClients = new PerSocketServiceTypeClients();
         this.mdnsFeatureFlags = mdnsFeatureFlags;
         this.discoveryExecutor = new DiscoveryExecutor(socketClient.getLooper());
-
-        // The very first posted task is to initialize the service cache, so it will be always
-        // set in other tasks running on the handler
-        discoveryExecutor.execute(() ->
-                serviceCache = new MdnsServiceCache(Looper.myLooper(), mdnsFeatureFlags));
     }
 
     private static class DiscoveryExecutor implements Executor {
@@ -358,9 +354,13 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback {
         sharedLog.log("createServiceTypeClient for type:" + serviceType + " " + socketKey);
         final String tag = serviceType + "-" + socketKey.getNetwork()
                 + "/" + socketKey.getInterfaceIndex();
+        final Looper looper = Looper.myLooper();
+        if (serviceCache == null) {
+            serviceCache = new MdnsServiceCache(looper, mdnsFeatureFlags);
+        }
         return new MdnsServiceTypeClient(
                 serviceType, socketClient,
                 executorProvider.newServiceTypeClientSchedulerExecutor(), socketKey,
-                sharedLog.forSubComponent(tag), Looper.myLooper(), serviceCache);
+                sharedLog.forSubComponent(tag), looper, serviceCache);
     }
 }
\ No newline at end of file