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

Commit 152ac326 authored by Amit Mahajan's avatar Amit Mahajan Committed by Android (Google) Code Review
Browse files

Merge "Fix the synchronization pattern in IccSmsInterfaceManager." into rvc-qpr-dev-plus-aosp

parents 194d300d e650b495
Loading
Loading
Loading
Loading
+116 −141
Original line number Diff line number Diff line
@@ -59,6 +59,7 @@ import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;

/**
 * IccSmsInterfaceManager to provide an inter-process communication to
@@ -68,15 +69,6 @@ public class IccSmsInterfaceManager {
    static final String LOG_TAG = "IccSmsInterfaceManager";
    static final boolean DBG = true;

    @UnsupportedAppUsage
    protected final Object mLock = new Object();
    @UnsupportedAppUsage
    protected boolean mSuccess;
    @UnsupportedAppUsage
    private List<SmsRawData> mSms;

    private String mSmsc;

    @UnsupportedAppUsage
    private CellBroadcastRangeManager mCellBroadcastRangeManager =
            new CellBroadcastRangeManager();
@@ -106,75 +98,80 @@ public class IccSmsInterfaceManager {

    private final LocalLog mCellBroadcastLocalLog = new LocalLog(100);

    private static final class Request {
        AtomicBoolean mStatus = new AtomicBoolean(false);
        Object mResult = null;
    }

    @UnsupportedAppUsage
    protected Handler mHandler = new Handler() {
        @Override
        public void handleMessage(Message msg) {
            AsyncResult ar;
            AsyncResult ar = (AsyncResult) msg.obj;
            Request request = (Request) ar.userObj;

            switch (msg.what) {
                case EVENT_UPDATE_DONE:
                    ar = (AsyncResult) msg.obj;
                    synchronized (mLock) {
                        mSuccess = (ar.exception == null);
                        mLock.notifyAll();
                    }
                case EVENT_SET_BROADCAST_ACTIVATION_DONE:
                case EVENT_SET_BROADCAST_CONFIG_DONE:
                case EVENT_SET_SMSC_DONE:
                    notifyPending(request, ar.exception == null);
                    break;
                case EVENT_LOAD_DONE:
                    ar = (AsyncResult)msg.obj;
                    synchronized (mLock) {
                    List<SmsRawData> smsRawDataList = null;
                    if (ar.exception == null) {
                            mSms = buildValidRawData((ArrayList<byte[]>) ar.result);
                        smsRawDataList = buildValidRawData((ArrayList<byte[]>) ar.result);
                        //Mark SMS as read after importing it from card.
                        markMessagesAsRead((ArrayList<byte[]>) ar.result);
                    } else {
                        if (Rlog.isLoggable("SMS", Log.DEBUG)) {
                            loge("Cannot load Sms records");
                        }
                            mSms = null;
                        }
                        mLock.notifyAll();
                    }
                    break;
                case EVENT_SET_BROADCAST_ACTIVATION_DONE:
                case EVENT_SET_BROADCAST_CONFIG_DONE:
                    ar = (AsyncResult) msg.obj;
                    synchronized (mLock) {
                        mSuccess = (ar.exception == null);
                        mLock.notifyAll();
                    }
                    notifyPending(request, smsRawDataList);
                    break;
                case EVENT_GET_SMSC_DONE:
                    ar = (AsyncResult) msg.obj;
                    synchronized (mLock) {
                    String smsc = null;
                    if (ar.exception == null) {
                            mSmsc = (String) ar.result;
                        smsc = (String) ar.result;
                    } else {
                        loge("Cannot read SMSC");
                            mSmsc = null;
                        }
                        mLock.notifyAll();
                    }
                    notifyPending(request, smsc);
                    break;
                case EVENT_SET_SMSC_DONE:
                    ar = (AsyncResult) msg.obj;
                    synchronized (mLock) {
                        mSuccess = (ar.exception == null);
                        mLock.notifyAll();
            }
                    break;
        }

        private void notifyPending(Request request, Object result) {
            if (request != null) {
                synchronized (request) {
                    request.mResult = result;
                    request.mStatus.set(true);
                    request.notifyAll();
                }
            }
        }
    };

    protected IccSmsInterfaceManager(Phone phone) {
        mPhone = phone;
        mContext = phone.getContext();
        mAppOps = (AppOpsManager) mContext.getSystemService(Context.APP_OPS_SERVICE);
        mDispatchersController =
        this(phone, phone.getContext(),
                (AppOpsManager) phone.getContext().getSystemService(Context.APP_OPS_SERVICE),
                new SmsDispatchersController(
                        phone, phone.mSmsStorageMonitor, phone.mSmsUsageMonitor);
        mSmsPermissions = new SmsPermissions(phone, mContext, mAppOps);
                        phone, phone.mSmsStorageMonitor, phone.mSmsUsageMonitor),
                new SmsPermissions(phone, phone.getContext(),
                        (AppOpsManager) phone.getContext().getSystemService(
                                Context.APP_OPS_SERVICE)));
    }

    @VisibleForTesting
    public IccSmsInterfaceManager(
            Phone phone, Context context, AppOpsManager appOps,
            SmsDispatchersController dispatchersController, SmsPermissions smsPermissions) {
        mPhone = phone;
        mContext = context;
        mAppOps = appOps;
        mDispatchersController = dispatchersController;
        mSmsPermissions = smsPermissions;
    }

    private void enforceNotOnHandlerThread(String methodName) {
@@ -258,9 +255,9 @@ public class IccSmsInterfaceManager {
                callingPackage) != AppOpsManager.MODE_ALLOWED) {
            return false;
        }
        synchronized(mLock) {
            mSuccess = false;
            Message response = mHandler.obtainMessage(EVENT_UPDATE_DONE);
        Request updateRequest = new Request();
        synchronized (updateRequest) {
            Message response = mHandler.obtainMessage(EVENT_UPDATE_DONE, updateRequest);

            if ((status & 0x01) == STATUS_ON_ICC_FREE) {
                // RIL_REQUEST_DELETE_SMS_ON_SIM vs RIL_REQUEST_CDMA_DELETE_SMS_ON_RUIM
@@ -277,20 +274,16 @@ public class IccSmsInterfaceManager {
                IccFileHandler fh = mPhone.getIccFileHandler();
                if (fh == null) {
                    response.recycle();
                    return mSuccess; /* is false */
                    return false; /* is false */
                }
                byte[] record = makeSmsRecordData(status, pdu);
                fh.updateEFLinearFixed(
                        IccConstants.EF_SMS,
                        index, record, null, response);
            }
            try {
                mLock.wait();
            } catch (InterruptedException e) {
                loge("interrupted while trying to update by index");
            }
            waitForResult(updateRequest);
        }
        return mSuccess;
        return (boolean) updateRequest.mResult;
    }

    /**
@@ -318,9 +311,9 @@ public class IccSmsInterfaceManager {
                callingPackage) != AppOpsManager.MODE_ALLOWED) {
            return false;
        }
        synchronized(mLock) {
            mSuccess = false;
            Message response = mHandler.obtainMessage(EVENT_UPDATE_DONE);
        Request copyRequest = new Request();
        synchronized (copyRequest) {
            Message response = mHandler.obtainMessage(EVENT_UPDATE_DONE, copyRequest);

            //RIL_REQUEST_WRITE_SMS_TO_SIM vs RIL_REQUEST_CDMA_WRITE_SMS_TO_RUIM
            if (PhoneConstants.PHONE_TYPE_GSM == mPhone.getPhoneType()) {
@@ -330,13 +323,9 @@ public class IccSmsInterfaceManager {
                mPhone.mCi.writeSmsToRuim(status, pdu, response);
            }

            try {
                mLock.wait();
            } catch (InterruptedException e) {
                loge("interrupted while trying to update by index");
            waitForResult(copyRequest);
        }
        }
        return mSuccess;
        return (boolean) copyRequest.mResult;
    }

    /**
@@ -358,25 +347,21 @@ public class IccSmsInterfaceManager {
                callingPackage) != AppOpsManager.MODE_ALLOWED) {
            return new ArrayList<SmsRawData>();
        }
        synchronized(mLock) {
        Request getRequest = new Request();
        synchronized (getRequest) {

            IccFileHandler fh = mPhone.getIccFileHandler();
            if (fh == null) {
                loge("Cannot load Sms records. No icc card?");
                mSms = null;
                return mSms;
                return null;
            }

            Message response = mHandler.obtainMessage(EVENT_LOAD_DONE);
            Message response = mHandler.obtainMessage(EVENT_LOAD_DONE, getRequest);
            fh.loadEFLinearFixedAll(IccConstants.EF_SMS, response);

            try {
                mLock.wait();
            } catch (InterruptedException e) {
                loge("interrupted while trying to load from the Icc");
            waitForResult(getRequest);
        }
        }
        return mSms;
        return (List<SmsRawData>) getRequest.mResult;
    }

    /**
@@ -878,17 +863,13 @@ public class IccSmsInterfaceManager {
            return null;
        }
        enforceNotOnHandlerThread("getSmscAddressFromIccEf");
        synchronized (mLock) {
            mSmsc = null;
            Message response = mHandler.obtainMessage(EVENT_GET_SMSC_DONE);
        Request getRequest = new Request();
        synchronized (getRequest) {
            Message response = mHandler.obtainMessage(EVENT_GET_SMSC_DONE, getRequest);
            mPhone.mCi.getSmscAddress(response);
            try {
                mLock.wait();
            } catch (InterruptedException e) {
                loge("interrupted while trying to read SMSC");
            }
            waitForResult(getRequest);
        }
        return mSmsc;
        return (String) getRequest.mResult;
    }

    /**
@@ -902,17 +883,14 @@ public class IccSmsInterfaceManager {
                callingPackage, "setSmscAddressOnIccEf")) {
            return false;
        }
        synchronized (mLock) {
            mSuccess = false;
            Message response = mHandler.obtainMessage(EVENT_SET_SMSC_DONE);
        enforceNotOnHandlerThread("setSmscAddressOnIccEf");
        Request setRequest = new Request();
        synchronized (setRequest) {
            Message response = mHandler.obtainMessage(EVENT_SET_SMSC_DONE, setRequest);
            mPhone.mCi.setSmscAddress(smsc, response);
            try {
                mLock.wait();
            } catch (InterruptedException e) {
                loge("interrupted while trying to write SMSC");
            waitForResult(setRequest);
        }
        }
        return mSuccess;
        return (boolean) setRequest.mResult;
    }

    public boolean enableCellBroadcast(int messageIdentifier, int ranType) {
@@ -1164,20 +1142,16 @@ public class IccSmsInterfaceManager {
            log("Calling setGsmBroadcastConfig with " + configs.length + " configurations");
        }
        enforceNotOnHandlerThread("setCellBroadcastConfig");
        synchronized (mLock) {
            Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_CONFIG_DONE);
        Request setRequest = new Request();
        synchronized (setRequest) {
            Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_CONFIG_DONE, setRequest);

            mSuccess = false;
            mPhone.mCi.setGsmBroadcastConfig(configs, response);

            try {
                mLock.wait();
            } catch (InterruptedException e) {
                loge("interrupted while trying to set cell broadcast config");
            }
            waitForResult(setRequest);
        }

        return mSuccess;
        return (boolean) setRequest.mResult;
    }

    private boolean setCellBroadcastActivation(boolean activate) {
@@ -1186,20 +1160,16 @@ public class IccSmsInterfaceManager {
        }

        enforceNotOnHandlerThread("setCellBroadcastConfig");
        synchronized (mLock) {
            Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_ACTIVATION_DONE);
        Request setRequest = new Request();
        synchronized (setRequest) {
            Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_ACTIVATION_DONE,
                    setRequest);

            mSuccess = false;
            mPhone.mCi.setGsmBroadcastActivation(activate, response);

            try {
                mLock.wait();
            } catch (InterruptedException e) {
                loge("interrupted while trying to set cell broadcast activation");
            }
            waitForResult(setRequest);
        }

        return mSuccess;
        return (boolean) setRequest.mResult;
    }

    @UnsupportedAppUsage
@@ -1209,20 +1179,16 @@ public class IccSmsInterfaceManager {
        }

        enforceNotOnHandlerThread("setCdmaBroadcastConfig");
        synchronized (mLock) {
            Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_CONFIG_DONE);
        Request setRequest = new Request();
        synchronized (setRequest) {
            Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_CONFIG_DONE, setRequest);

            mSuccess = false;
            mPhone.mCi.setCdmaBroadcastConfig(configs, response);

            try {
                mLock.wait();
            } catch (InterruptedException e) {
                loge("interrupted while trying to set cdma broadcast config");
            }
            waitForResult(setRequest);
        }

        return mSuccess;
        return (boolean) setRequest.mResult;
    }

    private boolean setCdmaBroadcastActivation(boolean activate) {
@@ -1231,20 +1197,17 @@ public class IccSmsInterfaceManager {
        }

        enforceNotOnHandlerThread("setCdmaBroadcastActivation");
        synchronized (mLock) {
            Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_ACTIVATION_DONE);
        Request setRequest = new Request();
        synchronized (setRequest) {
            Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_ACTIVATION_DONE,
                    setRequest);

            mSuccess = false;
            mPhone.mCi.setCdmaBroadcastActivation(activate, response);

            try {
                mLock.wait();
            } catch (InterruptedException e) {
                loge("interrupted while trying to set cdma broadcast activation");
            }
            waitForResult(setRequest);
        }

        return mSuccess;
        return (boolean) setRequest.mResult;
    }

    @UnsupportedAppUsage
@@ -1517,6 +1480,18 @@ public class IccSmsInterfaceManager {
        return result != null ? result : destAddr;
    }

    private void waitForResult(Request request) {
        synchronized (request) {
            while (!request.mStatus.get()) {
                try {
                    request.wait();
                } catch (InterruptedException e) {
                    log("Interrupted while waiting for result");
                }
            }
        }
    }

    public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
        pw.println("Enabled GSM channels: " + mCellBroadcastRangeManager);
        pw.println("Enabled CDMA channels: " + mCdmaBroadcastRangeManager);
+5 −0
Original line number Diff line number Diff line
@@ -537,6 +537,11 @@ public class ContextFixture implements TestFixture<Context> {
            enforceCallingOrSelfPermission(permission, message);
        }

        @Override
        public void enforceCallingPermission(String permission, String message) {
            enforceCallingOrSelfPermission(permission, message);
        }

        @Override
        public int checkCallingOrSelfPermission(String permission) {
            if (mPermissionTable.contains(permission)
+145 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2020 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.internal.telephony;

import static com.android.internal.telephony.TelephonyTestUtils.waitForMs;

import android.os.AsyncResult;
import android.os.Message;
import android.testing.AndroidTestingRunner;
import android.testing.TestableLooper;

import androidx.test.filters.SmallTest;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

@RunWith(AndroidTestingRunner.class)
@TestableLooper.RunWithLooper
public class IccSmsInterfaceManagerTest extends TelephonyTest {
    // object under test
    private IccSmsInterfaceManager mIccSmsInterfaceManager;

    @Mock
    private SmsPermissions mMockSmsPermissions;

    @Before
    public void setUp() throws Exception {
        super.setUp(getClass().getSimpleName());
        mIccSmsInterfaceManager = new IccSmsInterfaceManager(mPhone, mContext, mAppOpsManager,
                mSmsDispatchersController, mMockSmsPermissions);
    }

    @After
    public void tearDown() throws Exception {
        super.tearDown();
    }

    @Test
    @SmallTest
    public void testSynchronization() throws Exception {
        mContextFixture.addCallingOrSelfPermission(android.Manifest.permission
                .RECEIVE_EMERGENCY_BROADCAST);
        when(mMockSmsPermissions.checkCallingOrSelfCanGetSmscAddress(anyString(), anyString()))
                .thenReturn(true);
        mSimulatedCommands.mSendSetGsmBroadcastConfigResponse = false;
        mSimulatedCommands.mSendGetSmscAddressResponse = false;
        CountDownLatch enableRangeLatch = new CountDownLatch(1);
        CountDownLatch getSmscLatch = new CountDownLatch(1);

        // call enableGsmBroadcastRange from first thread
        Thread enableRangeThread = new Thread(new Runnable() {
            @Override
            public void run() {
                mIccSmsInterfaceManager.enableGsmBroadcastRange(0, 5);
                enableRangeLatch.countDown();
            }
        });
        enableRangeThread.start();

        // call getSmscAddressFromIccEf from second thread
        Thread getSmscThread = new Thread(new Runnable() {
            @Override
            public void run() {
                mIccSmsInterfaceManager.getSmscAddressFromIccEf("calling package");
                getSmscLatch.countDown();
            }
        });
        getSmscThread.start();

        // wait for half a second to let the threads send their messages
        waitForMs(500);
        processAllMessages();

        // latch count should not be reduced until response is sent
        assertEquals("enableRangeLatch.getCount should be 1", 1, enableRangeLatch.getCount());
        assertEquals("getSmscLatch.getCount should be 1", 1, getSmscLatch.getCount());

        // send back response for first request and assert that only the first thread is unblocked
        ArgumentCaptor<Message> enableRangeCaptor = ArgumentCaptor.forClass(Message.class);
        verify(mSimulatedCommandsVerifier).setGsmBroadcastConfig(any(),
                enableRangeCaptor.capture());
        Message enableRangeResponse = enableRangeCaptor.getValue();
        AsyncResult.forMessage(enableRangeResponse);
        enableRangeResponse.sendToTarget();
        processAllMessages();

        // the response triggers setGsmBroadcastActivation and since that's on another thread
        // (enableRangeThread), wait for half a second for the response message of that to reach
        // the handler before calling processAllMessages(). Consider increasing this timeout if the
        // test fails.
        waitForMs(500);
        processAllMessages();

        try {
            assertEquals("enableRangeLatch.await should be true", true,
                    enableRangeLatch.await(5, TimeUnit.SECONDS));
        } catch (InterruptedException ie) {
            fail("enableRangeLatch.await interrupted");
        }
        assertEquals("getSmscLatch.getCount should still be 1", 1, getSmscLatch.getCount());

        // send back response for second request and assert that the second thread gets unblocked
        ArgumentCaptor<Message> getSmscCaptor = ArgumentCaptor.forClass(Message.class);
        verify(mSimulatedCommandsVerifier).getSmscAddress(getSmscCaptor.capture());
        Message getSmscResponse = getSmscCaptor.getValue();
        AsyncResult.forMessage(getSmscResponse);
        getSmscResponse.sendToTarget();
        processAllMessages();

        try {
            assertEquals("getSmscLatch.await should be true", true,
                    getSmscLatch.await(5, TimeUnit.SECONDS));
        } catch (InterruptedException ie) {
            fail("getSmscLatch.await interrupted");
        }
    }
}
+10 −2
Original line number Diff line number Diff line
@@ -149,6 +149,8 @@ public class SimulatedCommands extends BaseCommands
    public int mDefaultRoamingIndicator;
    public int mReasonForDenial;
    public int mMaxDataCalls;
    public boolean mSendSetGsmBroadcastConfigResponse = true;
    public boolean mSendGetSmscAddressResponse = true;

    private SignalStrength mSignalStrength;
    private List<CellInfo> mCellInfoList = null;
@@ -1246,8 +1248,11 @@ public class SimulatedCommands extends BaseCommands

    @Override
    public void getSmscAddress(Message result) {
        SimulatedCommandsVerifier.getInstance().getSmscAddress(result);
        if (mSendGetSmscAddressResponse) {
            unimplemented(result);
        }
    }

    @Override
    public void setSmscAddress(String address, Message result) {
@@ -1867,8 +1872,11 @@ public class SimulatedCommands extends BaseCommands

    @Override
    public void setGsmBroadcastConfig(SmsBroadcastConfigInfo[] config, Message response) {
        SimulatedCommandsVerifier.getInstance().setGsmBroadcastConfig(config, response);
        if (mSendSetGsmBroadcastConfigResponse) {
            unimplemented(response);
        }
    }

    @Override
    public void getGsmBroadcastConfig(Message response) {