From 819e19ea2ab4c8c9570715ccdd2bb6aee1875513 Mon Sep 17 00:00:00 2001 From: markchien Date: Wed, 29 Sep 2021 00:30:20 +0800 Subject: [PATCH] Unregister the tethering internal callback in finalize Bug: 177265744 Bug: 191798390 Bug: 187972579 Test: atest TetheringServiceTest Change-Id: Ie7f9535b923db5073a59329ead22546a54e6ef47 --- .../src/android/net/TetheringManager.java | 17 +++++++++++++++++ .../tethering/TetheringServiceTest.java | 3 +-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/Tethering/common/TetheringLib/src/android/net/TetheringManager.java b/Tethering/common/TetheringLib/src/android/net/TetheringManager.java index 3086da2cee..4387ab0b26 100644 --- a/Tethering/common/TetheringLib/src/android/net/TetheringManager.java +++ b/Tethering/common/TetheringLib/src/android/net/TetheringManager.java @@ -290,6 +290,23 @@ public class TetheringManager { getConnector(c -> c.registerTetheringEventCallback(mCallback, pkgName)); } + /** @hide */ + @Override + protected void finalize() throws Throwable { + final String pkgName = mContext.getOpPackageName(); + Log.i(TAG, "unregisterTetheringEventCallback:" + pkgName); + // 1. It's generally not recommended to perform long operations in finalize, but while + // unregisterTetheringEventCallback does an IPC, it's a oneway IPC so should not block. + // 2. If the connector is not yet connected, TetheringManager is impossible to finalize + // because the connector polling thread strong reference the TetheringManager object. So + // it's guaranteed that registerTetheringEventCallback was already called before calling + // unregisterTetheringEventCallback in finalize. + if (mConnector == null) Log.wtf(TAG, "null connector in finalize!"); + getConnector(c -> c.unregisterTetheringEventCallback(mCallback, pkgName)); + + super.finalize(); + } + private void startPollingForConnector() { new Thread(() -> { while (true) { diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java index b7f5872967..f664d5d615 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java @@ -577,8 +577,7 @@ public final class TetheringServiceTest { assertNull("TetheringManager weak reference still not null after " + attempts + " attempts", weakTm.get()); - // BUG: internal callback do not be unregistered after TetheringManager is GCed. - assertEquals(1, callbacks.size()); + assertEquals("Internal callback is not unregistered", 0, callbacks.size()); }); } }