From 52e0dafcb3cd7043ee53220d3785c3056491213e Mon Sep 17 00:00:00 2001 From: Ahaan Ugale Date: Tue, 15 Mar 2022 01:13:13 -0700 Subject: [PATCH] Fix ServiceConnector failing unbind if not connected. ServiceConnector does not call unbindService when requested to unbind if the service isn't currently connected. This causes issues like leaving zombie Trusted Hotword processes bound forever if the process is stopped ('restarted') immediately after being created (say if audio server crashes twice in quick succession). Fix: 223845998 Test: manual - locally comment out code that immediately connects the service, then `for i in {1..10}; do adb shell cmd voiceinteraction\ restart-detection; done` - without fix results in an extra process Change-Id: I6a8c01390130bcec9aff1460004343ca2b207031 --- .../android/internal/infra/ServiceConnector.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/core/java/com/android/internal/infra/ServiceConnector.java b/core/java/com/android/internal/infra/ServiceConnector.java index 9e07f973b3c41..cb162674eb167 100644 --- a/core/java/com/android/internal/infra/ServiceConnector.java +++ b/core/java/com/android/internal/infra/ServiceConnector.java @@ -31,6 +31,7 @@ import android.os.Looper; import android.os.RemoteException; import android.os.UserHandle; import android.util.Log; +import android.util.Slog; import java.io.PrintWriter; import java.util.ArrayDeque; @@ -562,10 +563,21 @@ public interface ServiceConnector { void unbindJobThread() { cancelTimeout(); I service = mService; + // TODO(b/224695239): This is actually checking wasConnected. Rename and/or fix + // implementation based on what this should actually be checking. At least the first + // check for calling unbind is the correct behavior, though. boolean wasBound = service != null; + if (wasBound || mBinding) { + try { + mContext.unbindService(mServiceConnection); + } catch (IllegalArgumentException e) { // TODO(b/224697137): Fix the race condition + // that requires catching this (crashes if + // service isn't currently bound). + Slog.e(LOG_TAG, "Failed to unbind: " + e); + } + } if (wasBound) { dispatchOnServiceConnectionStatusChanged(service, false); - mContext.unbindService(mServiceConnection); service.asBinder().unlinkToDeath(this, 0); mService = null; }