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

Commit be525fb5 authored by Tom Chan's avatar Tom Chan
Browse files

Fix deadlock between attachSystemDataTransport and addOnTransportsChangedListener. 

Thread 1: CompanionTransportManager#attachSystemDataTransport -> (hold mTransports lock) -> notifyOnTransportsChanged -> (wait on mTransportsListeners)
Thread 2: CompanionTransportManager#addListener(IOnTransportsChangedListener listener) -> (hold mTransportsListeners lock) -> getAssociationsWithTransport -> (wait on mTransports)

This change is to use the same lock for both mTransports and
mTransportsListeners.

Test: Can no longer reproduce the deadlock in a test I am writing for wearable sensing.
Bug: 374165899
Flag: EXEMPT bug fix
Change-Id: Ife7fec38d163d81c9039e84406b2d12dcef518c8
parent 19ec5dc4

services/companion/java/com/android/server/companion/transport/CompanionTransportManager.java

+7 −4
Original line number Diff line number Diff line
@@ -57,8 +57,11 @@ public class CompanionTransportManager { 
    /** Association id -> Transport */

    @GuardedBy("mTransports")

    private final SparseArray<Transport> mTransports = new SparseArray<>();



    // Use mTransports to synchronize both mTransports and mTransportsListeners to avoid deadlock

    // between threads that access both

    @NonNull

    @GuardedBy("mTransportsListeners")

    @GuardedBy("mTransports")

    private final RemoteCallbackList<IOnTransportsChangedListener> mTransportsListeners =

            new RemoteCallbackList<>();
@@ -95,7 +98,7 @@ public class CompanionTransportManager { 
     */

    public void addListener(IOnTransportsChangedListener listener) {

        Slog.i(TAG, "Registering OnTransportsChangedListener");

        synchronized (mTransportsListeners) {

        synchronized (mTransports) {

            mTransportsListeners.register(listener);

            mTransportsListeners.broadcast(listener1 -> {

                // callback to the current listener with all the associations of the transports
@@ -114,7 +117,7 @@ public class CompanionTransportManager { 
     * Remove the listener for receiving callbacks when any of the transports is changed

     */

    public void removeListener(IOnTransportsChangedListener listener) {

        synchronized (mTransportsListeners) {

        synchronized (mTransports) {

            mTransportsListeners.unregister(listener);

        }

    }
@@ -204,7 +207,7 @@ public class CompanionTransportManager { 
    }



    private void notifyOnTransportsChanged() {

        synchronized (mTransportsListeners) {

        synchronized (mTransports) {

            mTransportsListeners.broadcast(listener -> {

                try {

                    listener.onTransportsChanged(getAssociationsWithTransport());