Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 403deac0 authored by William Escande's avatar William Escande Committed by Gerrit Code Review
Browse files

Merge changes Ifa029c35,I370a549a into main

* changes:
  Database: Reduce scope of the cache lock
  Database: release cache lock before callback
parents 016e262f 78a3681b
Loading
Loading
Loading
Loading
+114 −112
Original line number Diff line number Diff line
@@ -230,7 +230,6 @@ public class DatabaseManager {
     */
    @VisibleForTesting
    public boolean setCustomMeta(BluetoothDevice device, int key, byte[] newValue) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "setCustomMeta: device is null");
            return false;
@@ -241,6 +240,7 @@ public class DatabaseManager {
        }

        String address = device.getAddress();
        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                createMetadata(address, false);
            }
@@ -255,16 +255,15 @@ public class DatabaseManager {
            data.setCustomizedMeta(key, newValue);

            updateDatabase(data);
        }
        mAdapterService.metadataChanged(address, key, newValue);
        return true;
    }
    }

    /**
     * Get customized metadata from database with requested key
     */
    public byte[] getCustomMeta(BluetoothDevice device, int key) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "getCustomMeta: device is null");
            return null;
@@ -276,6 +275,7 @@ public class DatabaseManager {

        String address = device.getAddress();

        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                Log.d(TAG, "getCustomMeta: device " + device + " is not in cache");
                return null;
@@ -292,13 +292,13 @@ public class DatabaseManager {
    @VisibleForTesting
    public boolean setAudioPolicyMetadata(BluetoothDevice device,
            BluetoothSinkAudioPolicy policies) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "setAudioPolicyMetadata: device is null");
            return false;
        }

        String address = device.getAddress();
        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                createMetadata(address, false);
            }
@@ -318,7 +318,6 @@ public class DatabaseManager {
     */
    @VisibleForTesting
    public BluetoothSinkAudioPolicy getAudioPolicyMetadata(BluetoothDevice device) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "getAudioPolicyMetadata: device is null");
            return null;
@@ -326,6 +325,7 @@ public class DatabaseManager {

        String address = device.getAddress();

        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                Log.d(TAG, "getAudioPolicyMetadata: device " + device + " is not in cache");
                return null;
@@ -361,7 +361,6 @@ public class DatabaseManager {
    @VisibleForTesting
    public boolean setProfileConnectionPolicy(BluetoothDevice device, int profile,
            int newConnectionPolicy) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "setProfileConnectionPolicy: device is null");
            return false;
@@ -370,12 +369,15 @@ public class DatabaseManager {
        if (newConnectionPolicy != BluetoothProfile.CONNECTION_POLICY_UNKNOWN
                && newConnectionPolicy != BluetoothProfile.CONNECTION_POLICY_FORBIDDEN
                && newConnectionPolicy != BluetoothProfile.CONNECTION_POLICY_ALLOWED) {
                Log.e(TAG, "setProfileConnectionPolicy: invalid connection policy "
                        + newConnectionPolicy);
            Log.e(
                    TAG,
                    "setProfileConnectionPolicy: invalid connection policy " + newConnectionPolicy);
            return false;
        }

        String address = device.getAddress();

        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                if (newConnectionPolicy == BluetoothProfile.CONNECTION_POLICY_UNKNOWN) {
                    return true;
@@ -420,7 +422,6 @@ public class DatabaseManager {
     * {@link BluetoothProfile.CONNECTION_POLICY_ALLOWED}
     */
    public int getProfileConnectionPolicy(BluetoothDevice device, int profile) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "getProfileConnectionPolicy: device is null");
            return BluetoothProfile.CONNECTION_POLICY_UNKNOWN;
@@ -428,6 +429,7 @@ public class DatabaseManager {

        String address = device.getAddress();

        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                Log.d(TAG, "getProfileConnectionPolicy: device " + device.getAnonymizedAddress()
                        + " is not in cache");
@@ -455,7 +457,6 @@ public class DatabaseManager {
     */
    @VisibleForTesting
    public void setA2dpSupportsOptionalCodecs(BluetoothDevice device, int newValue) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "setA2dpOptionalCodec: device is null");
            return;
@@ -469,6 +470,7 @@ public class DatabaseManager {

        String address = device.getAddress();

        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                return;
            }
@@ -497,7 +499,6 @@ public class DatabaseManager {
    @VisibleForTesting
    @OptionalCodecsSupportStatus
    public int getA2dpSupportsOptionalCodecs(BluetoothDevice device) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "setA2dpOptionalCodec: device is null");
            return BluetoothA2dp.OPTIONAL_CODECS_SUPPORT_UNKNOWN;
@@ -505,6 +506,7 @@ public class DatabaseManager {

        String address = device.getAddress();

        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                Log.d(TAG, "getA2dpOptionalCodec: device " + device + " is not in cache");
                return BluetoothA2dp.OPTIONAL_CODECS_SUPPORT_UNKNOWN;
@@ -526,7 +528,6 @@ public class DatabaseManager {
     */
    @VisibleForTesting
    public void setA2dpOptionalCodecsEnabled(BluetoothDevice device, int newValue) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "setA2dpOptionalCodecEnabled: device is null");
            return;
@@ -540,6 +541,7 @@ public class DatabaseManager {

        String address = device.getAddress();

        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                return;
            }
@@ -568,14 +570,13 @@ public class DatabaseManager {
    @VisibleForTesting
    @OptionalCodecsPreferenceStatus
    public int getA2dpOptionalCodecsEnabled(BluetoothDevice device) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "getA2dpOptionalCodecEnabled: device is null");
            return BluetoothA2dp.OPTIONAL_CODECS_PREF_UNKNOWN;
        }

        String address = device.getAddress();

        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                Log.d(TAG, "getA2dpOptionalCodecEnabled: device " + device + " is not in cache");
                return BluetoothA2dp.OPTIONAL_CODECS_PREF_UNKNOWN;
@@ -977,22 +978,25 @@ public class DatabaseManager {
     * @return a Bundle containing the preferred audio profiles
     */
    public Bundle getPreferredAudioProfiles(BluetoothDevice device) {
        synchronized (mMetadataCache) {
        if (device == null) {
            Log.e(TAG, "getPreferredAudioProfiles: device is null");
            throw new IllegalArgumentException("getPreferredAudioProfiles: device is null");
        }

        String address = device.getAddress();
        final int outputOnlyProfile;
        final int duplexProfile;

        synchronized (mMetadataCache) {
            if (!mMetadataCache.containsKey(address)) {
                return Bundle.EMPTY;
            }

            // Gets the preferred audio profiles for each audio mode
            Metadata metadata = mMetadataCache.get(address);
            int outputOnlyProfile = metadata.preferred_output_only_profile;
            int duplexProfile = metadata.preferred_duplex_profile;
            outputOnlyProfile = metadata.preferred_output_only_profile;
            duplexProfile = metadata.preferred_duplex_profile;
        }

        // Checks if the default values are present (aka no explicit preference)
        if (outputOnlyProfile == 0 && duplexProfile == 0) {
@@ -1001,8 +1005,7 @@ public class DatabaseManager {

        Bundle modeToProfileBundle = new Bundle();
        if (outputOnlyProfile != 0) {
                modeToProfileBundle.putInt(
                        BluetoothAdapter.AUDIO_MODE_OUTPUT_ONLY, outputOnlyProfile);
            modeToProfileBundle.putInt(BluetoothAdapter.AUDIO_MODE_OUTPUT_ONLY, outputOnlyProfile);
        }
        if (duplexProfile != 0) {
            modeToProfileBundle.putInt(BluetoothAdapter.AUDIO_MODE_DUPLEX, duplexProfile);
@@ -1010,7 +1013,6 @@ public class DatabaseManager {

        return modeToProfileBundle;
    }
    }

    /**
     * Get the {@link Looper} for the handler thread. This is used in testing and helper
+35 −0
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.times;
@@ -50,6 +51,8 @@ import com.android.bluetooth.btservice.AdapterService;
import com.android.bluetooth.flags.FakeFeatureFlagsImpl;
import com.android.bluetooth.flags.Flags;

import com.google.common.truth.Truth;

import org.hamcrest.CoreMatchers;
import org.junit.After;
import org.junit.Assert;
@@ -59,10 +62,13 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.stubbing.Answer;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;

@MediumTest
@RunWith(AndroidJUnit4.class)
@@ -1687,4 +1693,33 @@ public final class DatabaseManagerTest {
        // Wait for clear database
        TestUtils.waitForLooperToFinishScheduledTask(mDatabaseManager.getHandlerLooper());
    }

    @Test
    public void setCustomMetadata_reentrantCallback_noDeadLock() throws Exception {
        final int key = 3;
        final byte[] newValue = new byte[2];

        CompletableFuture<byte[]> future = new CompletableFuture();

        Answer answer =
                invocation -> {
                    // Concurrent database call during callback execution
                    byte[] value =
                            CompletableFuture.supplyAsync(
                                            () -> mDatabaseManager.getCustomMeta(mTestDevice, key))
                                    .completeOnTimeout(null, 1, TimeUnit.SECONDS)
                                    .get();

                    future.complete(value);
                    return null;
                };

        doAnswer(answer)
                .when(mAdapterService)
                .metadataChanged(any(String.class), anyInt(), any(byte[].class));

        mDatabaseManager.setCustomMeta(mTestDevice, key, newValue);

        Truth.assertThat(future.get()).isEqualTo(newValue);
    }
}