Fix the synchronization pattern in IccSmsInterfaceManager. (e650b495) · Commits · e / os / android_frameworks_opt_telephony

src/java/com/android/internal/telephony/IccSmsInterfaceManager.java

+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);

tests/telephonytests/src/com/android/internal/telephony/ContextFixture.java

+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)

tests/telephonytests/src/com/android/internal/telephony/IccSmsInterfaceManagerTest.java

0 → 100644

+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");
		}
		}
		}

tests/telephonytests/src/com/android/internal/telephony/SimulatedCommands.java

+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) {