From 14f9771c72797dd269eb3cde43eca1ff64cd9097 Mon Sep 17 00:00:00 2001 From: shubang Date: Thu, 29 Jul 2021 10:20:43 -0700 Subject: [PATCH] Tuner APIs: add locks to runnables to avoid crashes caused by NPE The executor can be run in a different thread, which can cause race condition. Adding the lock can help avoid the NPE reported. Bug: 193604292 Test: atest android.media.tv.tuner.cts.TunerTest Change-Id: I647ea2b428c1510b1df7f6cecc686bb7eb288a18 --- media/java/android/media/tv/tuner/Lnb.java | 16 +- media/java/android/media/tv/tuner/Tuner.java | 160 +++++++++++++++--- .../media/tv/tuner/dvr/DvrPlayback.java | 8 +- .../media/tv/tuner/dvr/DvrRecorder.java | 8 +- .../android/media/tv/tuner/filter/Filter.java | 16 +- 5 files changed, 175 insertions(+), 33 deletions(-) diff --git a/media/java/android/media/tv/tuner/Lnb.java b/media/java/android/media/tv/tuner/Lnb.java index 8b69d33722c58..0037b53e541ab 100644 --- a/media/java/android/media/tv/tuner/Lnb.java +++ b/media/java/android/media/tv/tuner/Lnb.java @@ -175,7 +175,13 @@ public class Lnb implements AutoCloseable { private void onEvent(int eventType) { synchronized (mCallbackLock) { if (mExecutor != null && mCallback != null) { - mExecutor.execute(() -> mCallback.onEvent(eventType)); + mExecutor.execute(() -> { + synchronized (mCallbackLock) { + if (mCallback != null) { + mCallback.onEvent(eventType); + } + } + }); } } } @@ -183,7 +189,13 @@ public class Lnb implements AutoCloseable { private void onDiseqcMessage(byte[] diseqcMessage) { synchronized (mCallbackLock) { if (mExecutor != null && mCallback != null) { - mExecutor.execute(() -> mCallback.onDiseqcMessage(diseqcMessage)); + mExecutor.execute(() -> { + synchronized (mCallbackLock) { + if (mCallback != null) { + mCallback.onDiseqcMessage(diseqcMessage); + } + } + }); } } } diff --git a/media/java/android/media/tv/tuner/Tuner.java b/media/java/android/media/tv/tuner/Tuner.java index 325436648e63c..088b3b6d29498 100644 --- a/media/java/android/media/tv/tuner/Tuner.java +++ b/media/java/android/media/tv/tuner/Tuner.java @@ -629,8 +629,13 @@ public class Tuner implements AutoCloseable { synchronized (mOnResourceLostListenerLock) { if (mOnResourceLostListener != null && mOnResourceLostListenerExecutor != null) { - mOnResourceLostListenerExecutor.execute( - () -> mOnResourceLostListener.onResourceLost(Tuner.this)); + mOnResourceLostListenerExecutor.execute(() -> { + synchronized (mOnResourceLostListenerLock) { + if (mOnResourceLostListener != null) { + mOnResourceLostListener.onResourceLost(Tuner.this); + } + } + }); } } break; @@ -1068,7 +1073,13 @@ public class Tuner implements AutoCloseable { Log.d(TAG, "Got event from tuning. Event type: " + eventType); synchronized (mOnTuneEventLock) { if (mOnTuneEventExecutor != null && mOnTuneEventListener != null) { - mOnTuneEventExecutor.execute(() -> mOnTuneEventListener.onTuneEvent(eventType)); + mOnTuneEventExecutor.execute(() -> { + synchronized (mOnTuneEventLock) { + if (mOnTuneEventListener != null) { + mOnTuneEventListener.onTuneEvent(eventType); + } + } + }); } } @@ -1096,7 +1107,13 @@ public class Tuner implements AutoCloseable { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onLocked()); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onLocked(); + } + } + }); } } } @@ -1104,7 +1121,13 @@ public class Tuner implements AutoCloseable { private void onScanStopped() { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onScanStopped()); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onScanStopped(); + } + } + }); } } } @@ -1112,7 +1135,13 @@ public class Tuner implements AutoCloseable { private void onProgress(int percent) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onProgress(percent)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onProgress(percent); + } + } + }); } } } @@ -1120,7 +1149,13 @@ public class Tuner implements AutoCloseable { private void onFrequenciesReport(int[] frequency) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onFrequenciesReported(frequency)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onFrequenciesReported(frequency); + } + } + }); } } } @@ -1128,7 +1163,13 @@ public class Tuner implements AutoCloseable { private void onSymbolRates(int[] rate) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onSymbolRatesReported(rate)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onSymbolRatesReported(rate); + } + } + }); } } } @@ -1136,7 +1177,13 @@ public class Tuner implements AutoCloseable { private void onHierarchy(int hierarchy) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onHierarchyReported(hierarchy)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onHierarchyReported(hierarchy); + } + } + }); } } } @@ -1144,7 +1191,13 @@ public class Tuner implements AutoCloseable { private void onSignalType(int signalType) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onSignalTypeReported(signalType)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onSignalTypeReported(signalType); + } + } + }); } } } @@ -1152,7 +1205,13 @@ public class Tuner implements AutoCloseable { private void onPlpIds(int[] plpIds) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onPlpIdsReported(plpIds)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onPlpIdsReported(plpIds); + } + } + }); } } } @@ -1160,7 +1219,13 @@ public class Tuner implements AutoCloseable { private void onGroupIds(int[] groupIds) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onGroupIdsReported(groupIds)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onGroupIdsReported(groupIds); + } + } + }); } } } @@ -1168,8 +1233,13 @@ public class Tuner implements AutoCloseable { private void onInputStreamIds(int[] inputStreamIds) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute( - () -> mScanCallback.onInputStreamIdsReported(inputStreamIds)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onInputStreamIdsReported(inputStreamIds); + } + } + }); } } } @@ -1177,8 +1247,13 @@ public class Tuner implements AutoCloseable { private void onDvbsStandard(int dvbsStandandard) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute( - () -> mScanCallback.onDvbsStandardReported(dvbsStandandard)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onDvbsStandardReported(dvbsStandandard); + } + } + }); } } } @@ -1186,8 +1261,13 @@ public class Tuner implements AutoCloseable { private void onDvbtStandard(int dvbtStandard) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute( - () -> mScanCallback.onDvbtStandardReported(dvbtStandard)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onDvbtStandardReported(dvbtStandard); + } + } + }); } } } @@ -1195,7 +1275,13 @@ public class Tuner implements AutoCloseable { private void onAnalogSifStandard(int sif) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute(() -> mScanCallback.onAnalogSifStandardReported(sif)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onAnalogSifStandardReported(sif); + } + } + }); } } } @@ -1203,8 +1289,13 @@ public class Tuner implements AutoCloseable { private void onAtsc3PlpInfos(Atsc3PlpInfo[] atsc3PlpInfos) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute( - () -> mScanCallback.onAtsc3PlpInfosReported(atsc3PlpInfos)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onAtsc3PlpInfosReported(atsc3PlpInfos); + } + } + }); } } } @@ -1212,8 +1303,13 @@ public class Tuner implements AutoCloseable { private void onModulationReported(int modulation) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute( - () -> mScanCallback.onModulationReported(modulation)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onModulationReported(modulation); + } + } + }); } } } @@ -1221,8 +1317,13 @@ public class Tuner implements AutoCloseable { private void onPriorityReported(boolean isHighPriority) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute( - () -> mScanCallback.onPriorityReported(isHighPriority)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onPriorityReported(isHighPriority); + } + } + }); } } } @@ -1230,8 +1331,13 @@ public class Tuner implements AutoCloseable { private void onDvbcAnnexReported(int dvbcAnnex) { synchronized (mScanCallbackLock) { if (mScanCallbackExecutor != null && mScanCallback != null) { - mScanCallbackExecutor.execute( - () -> mScanCallback.onDvbcAnnexReported(dvbcAnnex)); + mScanCallbackExecutor.execute(() -> { + synchronized (mScanCallbackLock) { + if (mScanCallback != null) { + mScanCallback.onDvbcAnnexReported(dvbcAnnex); + } + } + }); } } } diff --git a/media/java/android/media/tv/tuner/dvr/DvrPlayback.java b/media/java/android/media/tv/tuner/dvr/DvrPlayback.java index 1f805d761d495..6c2b9e3b99f5d 100644 --- a/media/java/android/media/tv/tuner/dvr/DvrPlayback.java +++ b/media/java/android/media/tv/tuner/dvr/DvrPlayback.java @@ -119,7 +119,13 @@ public class DvrPlayback implements AutoCloseable { } synchronized (mListenerLock) { if (mExecutor != null && mListener != null) { - mExecutor.execute(() -> mListener.onPlaybackStatusChanged(status)); + mExecutor.execute(() -> { + synchronized (mListenerLock) { + if (mListener != null) { + mListener.onPlaybackStatusChanged(status); + } + } + }); } } } diff --git a/media/java/android/media/tv/tuner/dvr/DvrRecorder.java b/media/java/android/media/tv/tuner/dvr/DvrRecorder.java index 2b694668eb03c..212a71343a49d 100644 --- a/media/java/android/media/tv/tuner/dvr/DvrRecorder.java +++ b/media/java/android/media/tv/tuner/dvr/DvrRecorder.java @@ -82,7 +82,13 @@ public class DvrRecorder implements AutoCloseable { } synchronized (mListenerLock) { if (mExecutor != null && mListener != null) { - mExecutor.execute(() -> mListener.onRecordStatusChanged(status)); + mExecutor.execute(() -> { + synchronized (mListenerLock) { + if (mListener != null) { + mListener.onRecordStatusChanged(status); + } + } + }); } } } diff --git a/media/java/android/media/tv/tuner/filter/Filter.java b/media/java/android/media/tv/tuner/filter/Filter.java index 33742ffd99bfb..3db7047518b8d 100644 --- a/media/java/android/media/tv/tuner/filter/Filter.java +++ b/media/java/android/media/tv/tuner/filter/Filter.java @@ -256,7 +256,13 @@ public class Filter implements AutoCloseable { private void onFilterStatus(int status) { synchronized (mCallbackLock) { if (mCallback != null && mExecutor != null) { - mExecutor.execute(() -> mCallback.onFilterStatusChanged(this, status)); + mExecutor.execute(() -> { + synchronized (mCallbackLock) { + if (mCallback != null) { + mCallback.onFilterStatusChanged(this, status); + } + } + }); } } } @@ -264,7 +270,13 @@ public class Filter implements AutoCloseable { private void onFilterEvent(FilterEvent[] events) { synchronized (mCallbackLock) { if (mCallback != null && mExecutor != null) { - mExecutor.execute(() -> mCallback.onFilterEvent(this, events)); + mExecutor.execute(() -> { + synchronized (mCallbackLock) { + if (mCallback != null) { + mCallback.onFilterEvent(this, events); + } + } + }); } } }