Merge "Tuner APIs: add locks to avoid crashes caused by NPE" into sc-dev am: d7d85eb143

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/15290582 Change-Id: Id2beb939a4df428b438a0cae2df63f731faad2a0
2021-07-15 19:33:29 +00:00
parent 6605f0d148 d7d85eb143
commit 5b463f9b14
5 changed files with 186 additions and 107 deletions
--- a/media/java/android/media/tv/tuner/Lnb.java
+++ b/media/java/android/media/tv/tuner/Lnb.java
@@ -148,6 +148,7 @@ public class Lnb implements AutoCloseable {
    LnbCallback mCallback;
    Executor mExecutor;
    Tuner mTuner;
+    private final Object mCallbackLock = new Object();


    private native int nativeSetVoltage(int voltage);
@@ -164,20 +165,26 @@ public class Lnb implements AutoCloseable {
    private Lnb() {}

    void setCallback(Executor executor, @Nullable LnbCallback callback, Tuner tuner) {
-        mCallback = callback;
-        mExecutor = executor;
-        mTuner = tuner;
+        synchronized (mCallbackLock) {
+            mCallback = callback;
+            mExecutor = executor;
+            mTuner = tuner;
+        }
    }

    private void onEvent(int eventType) {
-        if (mExecutor != null && mCallback != null) {
-            mExecutor.execute(() -> mCallback.onEvent(eventType));
+        synchronized (mCallbackLock) {
+            if (mExecutor != null && mCallback != null) {
+                mExecutor.execute(() -> mCallback.onEvent(eventType));
+            }
        }
    }

    private void onDiseqcMessage(byte[] diseqcMessage) {
-        if (mExecutor != null && mCallback != null) {
-            mExecutor.execute(() -> mCallback.onDiseqcMessage(diseqcMessage));
+        synchronized (mCallbackLock) {
+            if (mExecutor != null && mCallback != null) {
+                mExecutor.execute(() -> mCallback.onDiseqcMessage(diseqcMessage));
+            }
        }
    }

--- a/media/java/android/media/tv/tuner/Tuner.java
+++ b/media/java/android/media/tv/tuner/Tuner.java
@@ -291,7 +291,7 @@ public class Tuner implements AutoCloseable  {
    @Nullable
    private OnTuneEventListener mOnTuneEventListener;
    @Nullable
-    private Executor mOnTunerEventExecutor;
+    private Executor mOnTuneEventExecutor;
    @Nullable
    private ScanCallback mScanCallback;
    @Nullable
@@ -301,6 +301,10 @@ public class Tuner implements AutoCloseable  {
    @Nullable
    private Executor mOnResourceLostListenerExecutor;

+    private final Object mOnTuneEventLock = new Object();
+    private final Object mScanCallbackLock = new Object();
+    private final Object mOnResourceLostListenerLock = new Object();
+
    private Integer mDemuxHandle;
    private Integer mFrontendCiCamHandle;
    private Integer mFrontendCiCamId;
@@ -398,18 +402,22 @@ public class Tuner implements AutoCloseable  {
     */
    public void setResourceLostListener(@NonNull @CallbackExecutor Executor executor,
            @NonNull OnResourceLostListener listener) {
-        Objects.requireNonNull(executor, "OnResourceLostListener must not be null");
-        Objects.requireNonNull(listener, "executor must not be null");
-        mOnResourceLostListener = listener;
-        mOnResourceLostListenerExecutor = executor;
+        synchronized (mOnResourceLostListenerLock) {
+            Objects.requireNonNull(executor, "OnResourceLostListener must not be null");
+            Objects.requireNonNull(listener, "executor must not be null");
+            mOnResourceLostListener = listener;
+            mOnResourceLostListenerExecutor = executor;
+        }
    }

    /**
     * Removes the listener for resource lost.
     */
    public void clearResourceLostListener() {
-        mOnResourceLostListener = null;
-        mOnResourceLostListenerExecutor = null;
+        synchronized (mOnResourceLostListenerLock) {
+            mOnResourceLostListener = null;
+            mOnResourceLostListenerExecutor = null;
+        }
    }

    /**
@@ -618,10 +626,12 @@ public class Tuner implements AutoCloseable  {
                    break;
                }
                case MSG_RESOURCE_LOST: {
-                    if (mOnResourceLostListener != null
+                    synchronized (mOnResourceLostListenerLock) {
+                        if (mOnResourceLostListener != null
                                && mOnResourceLostListenerExecutor != null) {
-                        mOnResourceLostListenerExecutor.execute(
-                                () -> mOnResourceLostListener.onResourceLost(Tuner.this));
+                            mOnResourceLostListenerExecutor.execute(
+                                    () -> mOnResourceLostListener.onResourceLost(Tuner.this));
+                        }
                    }
                    break;
                }
@@ -652,8 +662,10 @@ public class Tuner implements AutoCloseable  {
     */
    public void setOnTuneEventListener(@NonNull @CallbackExecutor Executor executor,
            @NonNull OnTuneEventListener eventListener) {
-        mOnTuneEventListener = eventListener;
-        mOnTunerEventExecutor = executor;
+        synchronized (mOnTuneEventLock) {
+            mOnTuneEventListener = eventListener;
+            mOnTuneEventExecutor = executor;
+        }
    }

    /**
@@ -663,9 +675,10 @@ public class Tuner implements AutoCloseable  {
     * @see #setOnTuneEventListener(Executor, OnTuneEventListener)
     */
    public void clearOnTuneEventListener() {
-        mOnTuneEventListener = null;
-        mOnTunerEventExecutor = null;
-
+        synchronized (mOnTuneEventLock) {
+            mOnTuneEventListener = null;
+            mOnTuneEventExecutor = null;
+        }
    }

    /**
@@ -747,32 +760,34 @@ public class Tuner implements AutoCloseable  {
    @Result
    public int scan(@NonNull FrontendSettings settings, @ScanType int scanType,
            @NonNull @CallbackExecutor Executor executor, @NonNull ScanCallback scanCallback) {
-        /**
-         * Scan can be called again for blink scan if scanCallback and executor are same as before.
-         */
-        if (((mScanCallback != null) && (mScanCallback != scanCallback))
+        synchronized (mScanCallbackLock) {
+            // Scan can be called again for blink scan if scanCallback and executor are same as
+            //before.
+            if (((mScanCallback != null) && (mScanCallback != scanCallback))
                || ((mScanCallbackExecutor != null) && (mScanCallbackExecutor != executor))) {
-            throw new IllegalStateException(
+                throw new IllegalStateException(
                    "Different Scan session already in progress.  stopScan must be called "
                        + "before a new scan session can be " + "started.");
-        }
-        mFrontendType = settings.getType();
-        if (mFrontendType == FrontendSettings.TYPE_DTMB) {
-            if (!TunerVersionChecker.checkHigherOrEqualVersionTo(
-                    TunerVersionChecker.TUNER_VERSION_1_1, "Scan with DTMB Frontend")) {
-                return RESULT_UNAVAILABLE;
            }
-        }
-        if (checkResource(TunerResourceManager.TUNER_RESOURCE_TYPE_FRONTEND)) {
-            mScanCallback = scanCallback;
-            mScanCallbackExecutor = executor;
-            mFrontendInfo = null;
-            FrameworkStatsLog
+            mFrontendType = settings.getType();
+            if (mFrontendType == FrontendSettings.TYPE_DTMB) {
+                if (!TunerVersionChecker.checkHigherOrEqualVersionTo(
+                        TunerVersionChecker.TUNER_VERSION_1_1,
+                        "Scan with DTMB Frontend")) {
+                    return RESULT_UNAVAILABLE;
+                }
+            }
+            if (checkResource(TunerResourceManager.TUNER_RESOURCE_TYPE_FRONTEND)) {
+                mScanCallback = scanCallback;
+                mScanCallbackExecutor = executor;
+                mFrontendInfo = null;
+                FrameworkStatsLog
                    .write(FrameworkStatsLog.TV_TUNER_STATE_CHANGED, mUserId,
                        FrameworkStatsLog.TV_TUNER_STATE_CHANGED__STATE__SCANNING);
-            return nativeScan(settings.getType(), settings, scanType);
+                return nativeScan(settings.getType(), settings, scanType);
+            }
+            return RESULT_UNAVAILABLE;
        }
-        return RESULT_UNAVAILABLE;
    }

    /**
@@ -788,14 +803,15 @@ public class Tuner implements AutoCloseable  {
     */
    @Result
    public int cancelScanning() {
-        FrameworkStatsLog
-                .write(FrameworkStatsLog.TV_TUNER_STATE_CHANGED, mUserId,
+        synchronized (mScanCallbackLock) {
+            FrameworkStatsLog.write(FrameworkStatsLog.TV_TUNER_STATE_CHANGED, mUserId,
                    FrameworkStatsLog.TV_TUNER_STATE_CHANGED__STATE__SCAN_STOPPED);

-        int retVal = nativeStopScan();
-        mScanCallback = null;
-        mScanCallbackExecutor = null;
-        return retVal;
+            int retVal = nativeStopScan();
+            mScanCallback = null;
+            mScanCallbackExecutor = null;
+            return retVal;
+        }
    }

    private boolean requestFrontend() {
@@ -1050,8 +1066,10 @@ public class Tuner implements AutoCloseable  {

    private void onFrontendEvent(int eventType) {
        Log.d(TAG, "Got event from tuning. Event type: " + eventType);
-        if (mOnTunerEventExecutor != null && mOnTuneEventListener != null) {
-            mOnTunerEventExecutor.execute(() -> mOnTuneEventListener.onTuneEvent(eventType));
+        synchronized (mOnTuneEventLock) {
+            if (mOnTuneEventExecutor != null && mOnTuneEventListener != null) {
+                mOnTuneEventExecutor.execute(() -> mOnTuneEventListener.onTuneEvent(eventType));
+            }
        }

        Log.d(TAG, "Wrote Stats Log for the events from tuning.");
@@ -1072,114 +1090,149 @@ public class Tuner implements AutoCloseable  {

    private void onLocked() {
        Log.d(TAG, "Wrote Stats Log for locked event from scanning.");
-        FrameworkStatsLog
-                .write(FrameworkStatsLog.TV_TUNER_STATE_CHANGED, mUserId,
-                    FrameworkStatsLog.TV_TUNER_STATE_CHANGED__STATE__LOCKED);
+        FrameworkStatsLog.write(
+                FrameworkStatsLog.TV_TUNER_STATE_CHANGED, mUserId,
+                FrameworkStatsLog.TV_TUNER_STATE_CHANGED__STATE__LOCKED);

-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onLocked());
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onLocked());
+            }
        }
    }

    private void onScanStopped() {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onScanStopped());
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onScanStopped());
+            }
        }
    }

    private void onProgress(int percent) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onProgress(percent));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onProgress(percent));
+            }
        }
    }

    private void onFrequenciesReport(int[] frequency) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onFrequenciesReported(frequency));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onFrequenciesReported(frequency));
+            }
        }
    }

    private void onSymbolRates(int[] rate) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onSymbolRatesReported(rate));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onSymbolRatesReported(rate));
+            }
        }
    }

    private void onHierarchy(int hierarchy) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onHierarchyReported(hierarchy));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onHierarchyReported(hierarchy));
+            }
        }
    }

    private void onSignalType(int signalType) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onSignalTypeReported(signalType));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onSignalTypeReported(signalType));
+            }
        }
    }

    private void onPlpIds(int[] plpIds) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onPlpIdsReported(plpIds));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onPlpIdsReported(plpIds));
+            }
        }
    }

    private void onGroupIds(int[] groupIds) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onGroupIdsReported(groupIds));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onGroupIdsReported(groupIds));
+            }
        }
    }

    private void onInputStreamIds(int[] inputStreamIds) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(
-                    () -> mScanCallback.onInputStreamIdsReported(inputStreamIds));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(
+                        () -> mScanCallback.onInputStreamIdsReported(inputStreamIds));
+            }
        }
    }

    private void onDvbsStandard(int dvbsStandandard) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(
-                    () -> mScanCallback.onDvbsStandardReported(dvbsStandandard));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(
+                        () -> mScanCallback.onDvbsStandardReported(dvbsStandandard));
+            }
        }
    }

    private void onDvbtStandard(int dvbtStandard) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onDvbtStandardReported(dvbtStandard));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(
+                        () -> mScanCallback.onDvbtStandardReported(dvbtStandard));
+            }
        }
    }

    private void onAnalogSifStandard(int sif) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(() -> mScanCallback.onAnalogSifStandardReported(sif));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(() -> mScanCallback.onAnalogSifStandardReported(sif));
+            }
        }
    }

    private void onAtsc3PlpInfos(Atsc3PlpInfo[] atsc3PlpInfos) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(
-                    () -> mScanCallback.onAtsc3PlpInfosReported(atsc3PlpInfos));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(
+                        () -> mScanCallback.onAtsc3PlpInfosReported(atsc3PlpInfos));
+            }
        }
    }

    private void onModulationReported(int modulation) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(
-                    () -> mScanCallback.onModulationReported(modulation));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(
+                        () -> mScanCallback.onModulationReported(modulation));
+            }
        }
    }

    private void onPriorityReported(boolean isHighPriority) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(
-                    () -> mScanCallback.onPriorityReported(isHighPriority));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(
+                        () -> mScanCallback.onPriorityReported(isHighPriority));
+            }
        }
    }

    private void onDvbcAnnexReported(int dvbcAnnex) {
-        if (mScanCallbackExecutor != null && mScanCallback != null) {
-            mScanCallbackExecutor.execute(
-                    () -> mScanCallback.onDvbcAnnexReported(dvbcAnnex));
+        synchronized (mScanCallbackLock) {
+            if (mScanCallbackExecutor != null && mScanCallback != null) {
+                mScanCallbackExecutor.execute(
+                        () -> mScanCallback.onDvbcAnnexReported(dvbcAnnex));
+            }
        }
    }

--- a/media/java/android/media/tv/tuner/dvr/DvrPlayback.java
+++ b/media/java/android/media/tv/tuner/dvr/DvrPlayback.java
@@ -85,6 +85,7 @@ public class DvrPlayback implements AutoCloseable {
    private static int sInstantId = 0;
    private int mSegmentId = 0;
    private int mUnderflow;
+    private final Object mListenerLock = new Object();

    private native int nativeAttachFilter(Filter filter);
    private native int nativeDetachFilter(Filter filter);
@@ -106,16 +107,20 @@ public class DvrPlayback implements AutoCloseable {
    /** @hide */
    public void setListener(
            @NonNull Executor executor, @NonNull OnPlaybackStatusChangedListener listener) {
-        mExecutor = executor;
-        mListener = listener;
+        synchronized (mListenerLock) {
+            mExecutor = executor;
+            mListener = listener;
+        }
    }

    private void onPlaybackStatusChanged(int status) {
        if (status == PLAYBACK_STATUS_EMPTY) {
            mUnderflow++;
        }
-        if (mExecutor != null && mListener != null) {
-            mExecutor.execute(() -> mListener.onPlaybackStatusChanged(status));
+        synchronized (mListenerLock) {
+            if (mExecutor != null && mListener != null) {
+                mExecutor.execute(() -> mListener.onPlaybackStatusChanged(status));
+            }
        }
    }

--- a/media/java/android/media/tv/tuner/dvr/DvrRecorder.java
+++ b/media/java/android/media/tv/tuner/dvr/DvrRecorder.java
@@ -48,6 +48,7 @@ public class DvrRecorder implements AutoCloseable {
    private int mSegmentId = 0;
    private int mOverflow;
    private Boolean mIsStopped = true;
+    private final Object mListenerLock = new Object();

    private native int nativeAttachFilter(Filter filter);
    private native int nativeDetachFilter(Filter filter);
@@ -69,16 +70,20 @@ public class DvrRecorder implements AutoCloseable {
    /** @hide */
    public void setListener(
            @NonNull Executor executor, @NonNull OnRecordStatusChangedListener listener) {
-        mExecutor = executor;
-        mListener = listener;
+        synchronized (mListenerLock) {
+            mExecutor = executor;
+            mListener = listener;
+        }
    }

    private void onRecordStatusChanged(int status) {
        if (status == Filter.STATUS_OVERFLOW) {
            mOverflow++;
        }
-        if (mExecutor != null && mListener != null) {
-            mExecutor.execute(() -> mListener.onRecordStatusChanged(status));
+        synchronized (mListenerLock) {
+            if (mExecutor != null && mListener != null) {
+                mExecutor.execute(() -> mListener.onRecordStatusChanged(status));
+            }
        }
    }

--- a/media/java/android/media/tv/tuner/filter/Filter.java
+++ b/media/java/android/media/tv/tuner/filter/Filter.java
@@ -227,6 +227,7 @@ public class Filter implements AutoCloseable {
    private long mNativeContext;
    private FilterCallback mCallback;
    private Executor mExecutor;
+    private final Object mCallbackLock = new Object();
    private final long mId;
    private int mMainType;
    private int mSubtype;
@@ -253,14 +254,18 @@ public class Filter implements AutoCloseable {
    }

    private void onFilterStatus(int status) {
-        if (mCallback != null && mExecutor != null) {
-            mExecutor.execute(() -> mCallback.onFilterStatusChanged(this, status));
+        synchronized (mCallbackLock) {
+            if (mCallback != null && mExecutor != null) {
+                mExecutor.execute(() -> mCallback.onFilterStatusChanged(this, status));
+            }
        }
    }

    private void onFilterEvent(FilterEvent[] events) {
-        if (mCallback != null && mExecutor != null) {
-            mExecutor.execute(() -> mCallback.onFilterEvent(this, events));
+        synchronized (mCallbackLock) {
+            if (mCallback != null && mExecutor != null) {
+                mExecutor.execute(() -> mCallback.onFilterEvent(this, events));
+            }
        }
    }

@@ -272,13 +277,17 @@ public class Filter implements AutoCloseable {

    /** @hide */
    public void setCallback(FilterCallback cb, Executor executor) {
-        mCallback = cb;
-        mExecutor = executor;
+        synchronized (mCallbackLock) {
+            mCallback = cb;
+            mExecutor = executor;
+        }
    }

    /** @hide */
    public FilterCallback getCallback() {
-        return mCallback;
+        synchronized (mCallbackLock) {
+            return mCallback;
+        }
    }

    /**