diff options
author | Mukesh Agrawal <quiche@google.com> | 2017-06-09 00:10:33 +0000 |
---|---|---|
committer | android-build-merger <android-build-merger@google.com> | 2017-06-09 00:10:33 +0000 |
commit | 91f98fddf00d3098c90e35816aed9cf45ad77dd3 (patch) | |
tree | 6ef367b4fc39e0daf59f5690cc3d9dd840d5eddb /service/java/com/android/server/wifi/WifiVendorHal.java | |
parent | 075bd7d36827139944a2f4e67f2c68fbb884d189 (diff) | |
parent | 221b119b6044721dfe3c43efe706ec3ef11ea9ff (diff) | |
download | wifi-91f98fddf00d3098c90e35816aed9cf45ad77dd3.tar.gz |
Merge "WifiVendorHal: improve handling of debug events" into oc-dev
am: 221b119b60
Change-Id: Icb61d0cdce6a8f35b624b309da19c1bb9314ad05
Diffstat (limited to 'service/java/com/android/server/wifi/WifiVendorHal.java')
-rw-r--r-- | service/java/com/android/server/wifi/WifiVendorHal.java | 60 |
1 files changed, 45 insertions, 15 deletions
diff --git a/service/java/com/android/server/wifi/WifiVendorHal.java b/service/java/com/android/server/wifi/WifiVendorHal.java index 3da5ef979..976eee4a9 100644 --- a/service/java/com/android/server/wifi/WifiVendorHal.java +++ b/service/java/com/android/server/wifi/WifiVendorHal.java @@ -64,6 +64,7 @@ import android.net.wifi.WifiManager; import android.net.wifi.WifiScanner; import android.net.wifi.WifiSsid; import android.net.wifi.WifiWakeReasonAndCounts; +import android.os.Handler; import android.os.Looper; import android.os.RemoteException; import android.util.MutableBoolean; @@ -204,16 +205,23 @@ public class WifiVendorHal { private IWifiRttController mIWifiRttController; private final HalDeviceManager mHalDeviceManager; private final HalDeviceManagerStatusListener mHalDeviceManagerStatusCallbacks; - private final Looper mLooper; private final IWifiStaIfaceEventCallback mIWifiStaIfaceEventCallback; private final IWifiChipEventCallback mIWifiChipEventCallback; private final RttEventCallback mRttEventCallback; + // Plumbing for event handling. + // + // Being final fields, they can be accessed without synchronization under + // some reasonable assumptions. See + // https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.5 + private final Looper mLooper; + private final Handler mHalEventHandler; public WifiVendorHal(HalDeviceManager halDeviceManager, Looper looper) { mHalDeviceManager = halDeviceManager; mLooper = looper; + mHalEventHandler = new Handler(looper); mHalDeviceManagerStatusCallbacks = new HalDeviceManagerStatusListener(); mIWifiStaIfaceEventCallback = new StaIfaceEventCallback(); mIWifiChipEventCallback = new ChipEventCallback(); @@ -2458,25 +2466,47 @@ public class WifiVendorHal { WifiDebugRingBufferStatus status, java.util.ArrayList<Byte> data) { //TODO(b/35875078) Reinstate logging when execessive callbacks are fixed // mVerboseLog.d("onDebugRingBufferDataAvailable " + status); - WifiNative.WifiLoggerEventHandler eventHandler; - synchronized (sLock) { - if (mLogEventHandler == null || status == null || data == null) return; - eventHandler = mLogEventHandler; - } - eventHandler.onRingBufferData( - ringBufferStatus(status), NativeUtil.byteArrayFromArrayList(data)); + mHalEventHandler.post(() -> { + WifiNative.WifiLoggerEventHandler eventHandler; + synchronized (sLock) { + if (mLogEventHandler == null || status == null || data == null) return; + eventHandler = mLogEventHandler; + } + // Because |sLock| has been released, there is a chance that we'll execute + // a spurious callback (after someone has called resetLogHandler()). + // + // However, the alternative risks deadlock. Consider: + // [T1.1] WifiDiagnostics.captureBugReport() + // [T1.2] -- acquire WifiDiagnostics object's intrinsic lock + // [T1.3] -> WifiVendorHal.getRingBufferData() + // [T1.4] -- acquire WifiVendorHal.sLock + // [T2.1] <lambda>() + // [T2.2] -- acquire WifiVendorHal.sLock + // [T2.3] -> WifiDiagnostics.onRingBufferData() + // [T2.4] -- acquire WifiDiagnostics object's intrinsic lock + // + // The problem here is that the two threads acquire the locks in opposite order. + // If, for example, T2.2 executes between T1.2 and 1.4, then T1 and T2 + // will be deadlocked. + eventHandler.onRingBufferData( + ringBufferStatus(status), NativeUtil.byteArrayFromArrayList(data)); + }); } @Override public void onDebugErrorAlert(int errorCode, java.util.ArrayList<Byte> debugData) { mVerboseLog.d("onDebugErrorAlert " + errorCode); - WifiNative.WifiLoggerEventHandler eventHandler; - synchronized (sLock) { - if (mLogEventHandler == null || debugData == null) return; - eventHandler = mLogEventHandler; - } - eventHandler.onWifiAlert( - errorCode, NativeUtil.byteArrayFromArrayList(debugData)); + mHalEventHandler.post(() -> { + WifiNative.WifiLoggerEventHandler eventHandler; + synchronized (sLock) { + if (mLogEventHandler == null || debugData == null) return; + eventHandler = mLogEventHandler; + } + // See comment in onDebugRingBufferDataAvailable(), for an explanation + // of why this callback is invoked without |sLock| held. + eventHandler.onWifiAlert( + errorCode, NativeUtil.byteArrayFromArrayList(debugData)); + }); } } |