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

Commit c0d2d3e7 authored by Amit Mahajan's avatar Amit Mahajan Committed by Automerger Merge Worker
Browse files

Merge "Fix the synchronization pattern in IccSmsInterfaceManager." am: 8076cbee

Original change: https://android-review.googlesource.com/c/platform/frameworks/opt/telephony/+/1427152

Change-Id: Ib40ab2c41cd3bf1038fe4b338158af4c6210e62e
parents c41398d1 8076cbee
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
@@ -535,6 +535,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) {