Fix deadlock in BaseDataProducer.

Move calls to abstract method outside the synchronized block. When they
are in the synchronized block it can cause a deadlock in the following
way. If two classes have locks and they interact through callbacks the
aquiring the locks can have a mixed order.  The mixed order causes a
deadlock.

Bug: 276436535
Test: Run foldable samples.
Change-Id: Ie71ec56e7ba43976fee3e69f74ff386c79c96a7a
This commit is contained in:
Diego Vela
2023-04-13 21:01:11 +00:00
parent 97f689db8a
commit 368b8b845f
3 changed files with 25 additions and 17 deletions

View File

@@ -39,7 +39,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
/**
@@ -167,14 +166,13 @@ public final class DeviceStateManagerFoldingFeatureProducer
}
@Override
protected void onListenersChanged(
@NonNull Set<Consumer<List<CommonFoldingFeature>>> callbacks) {
super.onListenersChanged(callbacks);
if (callbacks.isEmpty()) {
protected void onListenersChanged() {
super.onListenersChanged();
if (hasListeners()) {
mRawFoldSupplier.addDataChangedCallback(this::notifyFoldingFeatureChange);
} else {
mCurrentDeviceState = INVALID_DEVICE_STATE;
mRawFoldSupplier.removeDataChangedCallback(this::notifyFoldingFeatureChange);
} else {
mRawFoldSupplier.addDataChangedCallback(this::notifyFoldingFeatureChange);
}
}

View File

@@ -31,7 +31,6 @@ import androidx.window.util.BaseDataProducer;
import com.android.internal.R;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
/**
@@ -86,11 +85,11 @@ public final class RawFoldingFeatureProducer extends BaseDataProducer<String> {
}
@Override
protected void onListenersChanged(Set<Consumer<String>> callbacks) {
if (callbacks.isEmpty()) {
unregisterObserversIfNeeded();
} else {
protected void onListenersChanged() {
if (hasListeners()) {
registerObserversIfNeeded();
} else {
unregisterObserversIfNeeded();
}
}

View File

@@ -51,10 +51,10 @@ public abstract class BaseDataProducer<T> implements DataProducer<T>,
public final void addDataChangedCallback(@NonNull Consumer<T> callback) {
synchronized (mLock) {
mCallbacks.add(callback);
Optional<T> currentData = getCurrentData();
currentData.ifPresent(callback);
onListenersChanged(mCallbacks);
}
Optional<T> currentData = getCurrentData();
currentData.ifPresent(callback);
onListenersChanged();
}
/**
@@ -67,11 +67,22 @@ public abstract class BaseDataProducer<T> implements DataProducer<T>,
public final void removeDataChangedCallback(@NonNull Consumer<T> callback) {
synchronized (mLock) {
mCallbacks.remove(callback);
onListenersChanged(mCallbacks);
}
onListenersChanged();
}
/**
* Returns {@code true} if there are any registered callbacks {@code false} if there are no
* registered callbacks.
*/
// TODO(b/278132889) Improve the structure of BaseDataProdcuer while avoiding known issues.
public final boolean hasListeners() {
synchronized (mLock) {
return !mCallbacks.isEmpty();
}
}
protected void onListenersChanged(Set<Consumer<T>> callbacks) {}
protected void onListenersChanged() {}
/**
* @return the current data if available and {@code Optional.empty()} otherwise.