Merge "Remove flag: explicit_kill_from_system_server" into main (b937bca4) · Commits · e / os / android_packages_modules_Bluetooth

android/app/src/com/android/bluetooth/btservice/AdapterService.java

+6 −54

Original line number	Diff line number	Diff line
		@@ -794,28 +794,14 @@ public class AdapterService extends Service {
		}

		@Override
		@RequiresPermission(BLUETOOTH_CONNECT)
		public boolean onUnbind(Intent intent) {
		if (Flags.explicitKillFromSystemServer()) {
		Log.d(TAG, "onUnbind()");
		return super.onUnbind(intent);
		}
		Log.d(TAG, "onUnbind() - calling cleanup");
		cleanup();
		return super.onUnbind(intent);
		}

		@Override
		public void onDestroy() {
		Log.d(TAG, "onDestroy()");
		if (Flags.explicitKillFromSystemServer()) {
		return;
		}
		if (!isMock()) {
		// TODO(b/27859763)
		Log.i(TAG, "Force exit to cleanup internal state in Bluetooth stack");
		System.exit(0);
		}
		}

		public ActiveDeviceManager getActiveDeviceManager() {
		@@ -1484,14 +1470,6 @@ public class AdapterService extends Service {
		mBluetoothSocketManagerBinder = null;
		}

		if (!Flags.explicitKillFromSystemServer()) {
		// Bluetooth will be killed, no need to cleanup binder
		if (mBinder != null) {
		mBinder.cleanup();
		mBinder = null; // Do not remove. Otherwise Binder leak!
		}
		}

		mPreferredAudioProfilesCallbacks.kill();

		mBluetoothQualityReportReadyCallbacks.kill();
		@@ -2252,20 +2230,12 @@ public class AdapterService extends Service {
		}

		/**
		* The Binder implementation must be declared to be a static class, with the AdapterService
		* instance passed in the constructor. Furthermore, when the AdapterService shuts down, the
		* reference to the AdapterService must be explicitly removed.
		*
		* <p>Otherwise, a memory leak can occur from repeated starting/stopping the service...Please
		* refer to android.os.Binder for further details on why an inner instance class should be
		* avoided.
		*
		* <p>TODO: b/339548431 -- Delete this comment as it does not apply when we get killed
		* There is no leak of this binder since it is never re-used and the process is systematically
		* killed
		*/
		@VisibleForTesting
		public static class AdapterServiceBinder extends IBluetooth.Stub {
		// TODO: b/339548431 move variable to final
		private AdapterService mService;
		private final AdapterService mService;

		AdapterServiceBinder(AdapterService svc) {
		mService = svc;
		@@ -2273,18 +2243,11 @@ public class AdapterService extends Service {
		BluetoothAdapter.getDefaultAdapter().disableBluetoothGetStateCache();
		}

		public void cleanup() {
		mService = null;
		}

		public AdapterService getService() {
		// Cache mService because it can change while getService is called
		AdapterService service = mService;

		if (service == null \|\| !service.isAvailable()) {
		if (!mService.isAvailable()) {
		return null;
		}
		return service;
		return mService;
		}

		@Override
		@@ -6888,15 +6851,4 @@ public class AdapterService extends Service {
		mPhonePolicy.onUuidsDiscovered(device, uuids);
		}
		}

		// TODO: b/339548431 delete isMock
		// Returns if this is a mock object. This is currently used in testing so that we may not call
		// System.exit() while finalizing the object. Otherwise GC of mock objects unfortunately ends up
		// calling finalize() which in turn calls System.exit() and the process crashes.
		//
		// Mock this in your testing framework to return true to avoid the mentioned behavior. In
		// production this has no effect.
		public boolean isMock() {
		return false;
		}
		}

android/app/src/com/android/bluetooth/btservice/AdapterState.java

+0 −5

Original line number	Diff line number	Diff line
		@@ -22,7 +22,6 @@ import android.os.Message;
		import android.os.SystemProperties;
		import android.util.Log;

		import com.android.bluetooth.flags.Flags;
		import com.android.internal.util.State;
		import com.android.internal.util.StateMachine;

		@@ -172,10 +171,6 @@ final class AdapterState extends StateMachine {

		@Override
		public void enter() {
		if (!Flags.explicitKillFromSystemServer()) {
		super.enter();
		return;
		}
		int prevState = mPrevState;
		super.enter();
		if (prevState == BluetoothAdapter.STATE_BLE_TURNING_OFF) {

android/app/tests/unit/src/com/android/bluetooth/btservice/AdapterPropertiesTest.java

+0 −1

Original line number	Diff line number	Diff line
		@@ -89,7 +89,6 @@ public class AdapterPropertiesTest {
		mRemoteDevices.reset();

		doReturn(mHandlerThread.getLooper()).when(mAdapterService).getMainLooper();
		doReturn(true).when(mAdapterService).isMock();
		when(mAdapterService.getResources())
		.thenReturn(InstrumentationRegistry.getTargetContext().getResources());

android/app/tests/unit/src/com/android/bluetooth/btservice/AdapterServiceBinderTest.java

+17 −10

Original line number	Diff line number	Diff line
		@@ -27,7 +27,6 @@ import android.bluetooth.IBluetoothOobDataCallback;
		import android.content.AttributionSource;
		import android.os.ParcelUuid;

		import org.junit.After;
		import org.junit.Before;
		import org.junit.Rule;
		import org.junit.Test;
		@@ -56,11 +55,6 @@ public class AdapterServiceBinderTest {
		mAttributionSource = new AttributionSource.Builder(0).build();
		}

		@After
		public void cleanUp() {
		mBinder.cleanup();
		}

		@Test
		public void getAddress() {
		mBinder.getAddress(mAttributionSource);
		@@ -73,10 +67,16 @@ public class AdapterServiceBinderTest {
		String[] args = new String[] {};
		mBinder.dump(fd, args);
		verify(mService).dump(any(), any(), any());
		}

		@Test
		public void dumpWhenNotAvailable() {
		FileDescriptor fd = new FileDescriptor();
		String[] args = new String[] {};
		doReturn(false).when(mService).isAvailable();

		Mockito.clearInvocations(mService);
		mBinder.cleanup();
		mBinder.dump(fd, args);

		verify(mService, never()).dump(any(), any(), any());
		}

		@@ -86,11 +86,18 @@ public class AdapterServiceBinderTest {
		IBluetoothOobDataCallback cb = Mockito.mock(IBluetoothOobDataCallback.class);

		mBinder.generateLocalOobData(transport, cb, mAttributionSource);

		verify(mService).generateLocalOobData(transport, cb);
		}

		@Test
		public void generateLocalOobDataWhenNotAvailable() {
		int transport = 0;
		IBluetoothOobDataCallback cb = Mockito.mock(IBluetoothOobDataCallback.class);
		doReturn(false).when(mService).isAvailable();

		Mockito.clearInvocations(mService);
		mBinder.cleanup();
		mBinder.generateLocalOobData(transport, cb, mAttributionSource);

		verify(mService, never()).generateLocalOobData(transport, cb);
		}

android/app/tests/unit/src/com/android/bluetooth/btservice/AdapterServiceTest.java

+19 −37

Original line number	Diff line number	Diff line
		@@ -203,7 +203,7 @@ public class AdapterServiceTest {

		@Parameters(name = "{0}")
		public static List<FlagsParameterization> getParams() {
		return FlagsParameterization.allCombinationsOf(Flags.FLAG_EXPLICIT_KILL_FROM_SYSTEM_SERVER);
		return FlagsParameterization.allCombinationsOf();
		}

		public AdapterServiceTest(FlagsParameterization flags) {
		@@ -552,12 +552,9 @@ public class AdapterServiceTest {
		verify(nativeInterface).disable();
		adapter.stateChangeCallback(AbstractionLayer.BT_STATE_OFF);
		TestUtils.syncHandler(looper, AdapterState.BLE_STOPPED);
		if (Flags.explicitKillFromSystemServer()) {
		// When reaching the OFF state, the cleanup is called that will destroy the state
		// machine of the adapterService. Destroying state machine send a -1 event on the
		// handler
		// When reaching the OFF state, the cleanup is called that will destroy the state machine of
		// the adapterService. Destroying state machine send a -1 event on the handler
		TestUtils.syncHandler(looper, -1);
		}
		verifyStateChange(callback, STATE_BLE_TURNING_OFF, STATE_OFF);

		assertThat(adapter.getState()).isEqualTo(STATE_OFF);
		@@ -640,12 +637,9 @@ public class AdapterServiceTest {
		assertThat(mAdapterService.getBluetoothGatt()).isNull();

		syncHandler(AdapterState.BLE_STOPPED);
		if (Flags.explicitKillFromSystemServer()) {
		// When reaching the OFF state, the cleanup is called that will destroy the state
		// machine of the adapterService. Destroying state machine send a -1 event on the
		// handler
		// When reaching the OFF state, the cleanup is called that will destroy the state machine of
		// the adapterService. Destroying state machine send a -1 event on the handler
		syncHandler(-1);
		}
		syncHandler(MESSAGE_PROFILE_SERVICE_STATE_CHANGED);
		syncHandler(MESSAGE_PROFILE_SERVICE_UNREGISTERED);

		@@ -678,12 +672,9 @@ public class AdapterServiceTest {

		mLooper.moveTimeForward(120_000); // Skip time so the timeout fires
		syncHandler(AdapterState.BLE_STOP_TIMEOUT);
		if (Flags.explicitKillFromSystemServer()) {
		// When reaching the OFF state, the cleanup is called that will destroy the state
		// machine of the adapterService. Destroying state machine send a -1 event on the
		// handler
		// When reaching the OFF state, the cleanup is called that will destroy the state machine of
		// the adapterService. Destroying state machine send a -1 event on the handler
		syncHandler(-1);
		}
		verifyStateChange(STATE_BLE_TURNING_OFF, STATE_OFF);

		assertThat(mAdapterService.getState()).isEqualTo(STATE_OFF);
		@@ -739,12 +730,9 @@ public class AdapterServiceTest {
		verify(mNativeInterface).disable();
		mAdapterService.stateChangeCallback(AbstractionLayer.BT_STATE_OFF);
		syncHandler(AdapterState.BLE_STOPPED);
		if (Flags.explicitKillFromSystemServer()) {
		// When reaching the OFF state, the cleanup is called that will destroy the state
		// machine of the adapterService. Destroying state machine send a -1 event on the
		// handler
		// When reaching the OFF state, the cleanup is called that will destroy the state machine of
		// the adapterService. Destroying state machine send a -1 event on the handler
		syncHandler(-1);
		}
		verifyStateChange(callback, STATE_BLE_TURNING_OFF, STATE_OFF);

		assertThat(mAdapterService.getState()).isEqualTo(STATE_OFF);
		@@ -890,12 +878,9 @@ public class AdapterServiceTest {
		// TODO(b/280518177): The only timeout to fire here should be the BREDR
		mLooper.moveTimeForward(120_000); // Skip time so the timeout fires
		syncHandler(AdapterState.BLE_STOP_TIMEOUT);
		if (Flags.explicitKillFromSystemServer()) {
		// When reaching the OFF state, the cleanup is called that will destroy the state
		// machine of the adapterService. Destroying state machine send a -1 event on the
		// handler
		// When reaching the OFF state, the cleanup is called that will destroy the state machine of
		// the adapterService. Destroying state machine send a -1 event on the handler
		syncHandler(-1);
		}
		verifyStateChange(STATE_BLE_TURNING_OFF, STATE_OFF);

		assertThat(mAdapterService.getState()).isEqualTo(STATE_OFF);
		@@ -938,12 +923,9 @@ public class AdapterServiceTest {

		mAdapterService.stateChangeCallback(AbstractionLayer.BT_STATE_OFF);
		syncHandler(AdapterState.BLE_STOPPED);
		if (Flags.explicitKillFromSystemServer()) {
		// When reaching the OFF state, the cleanup is called that will destroy the state
		// machine of the adapterService. Destroying state machine send a -1 event on the
		// handler
		// When reaching the OFF state, the cleanup is called that will destroy the state machine of
		// the adapterService. Destroying state machine send a -1 event on the handler
		syncHandler(-1);
		}

		verifyStateChange(STATE_BLE_TURNING_OFF, STATE_OFF);
		assertThat(mAdapterService.getState()).isEqualTo(STATE_OFF);