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

Commit c0869621 authored by Hwangoo Park's avatar Hwangoo Park
Browse files

Fix null pointer exception when sending SMS at short intervals

The null pointer exception was observed when sending consecutive SMS
messages at short intervals by the race condition between the
finishDomainSelection and requestDomainSelection in
SmsDispatchersController. To protect against this null pointer
exception, the requestDomainSelection which is working on the execution
flow of SmsDispatchersController is responsible for creating a
DomainSelectionConnection and the fallback to the legacy implementation
is also modified to be handled in this creation logic.
This ensures that finishDomainSelection and requestDomainSelection are
processed synchronously.

Bug: 317070933
Test: atest SmsDispatchersControllerTest
Test: manual (verified short/long SMS/Emergency SMS when domain
selection enabled - b/339313390#comment6, b/339313390#comment8)
Test: manual (verified multiple SMS/Emergency SMS (100 times) -
b/339313390#comment10 ~ comment13)

Change-Id: Idfa1a607043ed84ed8a03171ccf5f040967cb6ed
parent 2316a64b
Loading
Loading
Loading
Loading
+58 −94
Original line number Diff line number Diff line
@@ -781,15 +781,10 @@ public class SmsDispatchersController extends Handler {
            if (isSmsDomainSelectionEnabled()) {
                TelephonyManager tm = mContext.getSystemService(TelephonyManager.class);
                boolean isEmergency = tm.isEmergencyNumber(tracker.mDestAddress);
                DomainSelectionConnectionHolder holder = getDomainSelectionConnection(isEmergency);

                // If the DomainSelectionConnection is not available,
                // fallback to the legacy implementation.
                if (holder != null && holder.getConnection() != null) {
                // This may be invoked by another thread, so this operation is posted and
                // handled through the execution flow of SmsDispatchersController.
                SomeArgs args = SomeArgs.obtain();
                    args.arg1 = holder;
                args.arg1 = getDomainSelectionConnectionHolder(isEmergency);
                args.arg2 = new PendingRequest(PendingRequest.TYPE_RETRY_SMS, tracker,
                        null, null, null, null, null, false, null, 0, null, null, false,
                        0, false, 0, 0L, false);
@@ -797,7 +792,6 @@ public class SmsDispatchersController extends Handler {
                sendMessage(obtainMessage(EVENT_REQUEST_DOMAIN_SELECTION, args));
                return;
            }
            }

            if (mImsSmsDispatcher.isAvailable()) {
                // If this tracker has not been handled by ImsSmsDispatcher yet and IMS Service is
@@ -1007,52 +1001,23 @@ public class SmsDispatchersController extends Handler {
     * Returns a {@link DomainSelectionConnectionHolder} according to the flag specified.
     *
     * @param emergency The flag to indicate that the domain selection is for an emergency SMS.
     * @return A {@link DomainSelectionConnectionHolder} instance or null.
     * @return A {@link DomainSelectionConnectionHolder} instance.
     */
    @VisibleForTesting
    @Nullable
    protected DomainSelectionConnectionHolder getDomainSelectionConnectionHolder(
            boolean emergency) {
        return emergency ? mEmergencyDscHolder : mDscHolder;
    }

    /**
     * Returns a {@link DomainSelectionConnectionHolder} if the domain selection supports,
     * return null otherwise.
     *
     * @param emergency The flag to indicate that the domain selection is for an emergency SMS.
     * @return A {@link DomainSelectionConnectionHolder} that grabs the
     *         {@link DomainSelectionConnection} and its related information to use the domain
     *         selection architecture.
     */
    private DomainSelectionConnectionHolder getDomainSelectionConnection(boolean emergency) {
        DomainSelectionConnectionHolder holder = getDomainSelectionConnectionHolder(emergency);
        DomainSelectionConnection connection = (holder != null) ? holder.getConnection() : null;

        if (connection == null) {
            connection = mDomainSelectionResolverProxy.getDomainSelectionConnection(
                    mPhone, DomainSelectionService.SELECTOR_TYPE_SMS, emergency);

            if (connection == null) {
                // Domain selection architecture is not supported.
                // Use the legacy architecture.
                return null;
            }
        }

        if (holder == null) {
            holder = new DomainSelectionConnectionHolder(emergency);

        if (emergency) {
                mEmergencyDscHolder = holder;
            if (mEmergencyDscHolder == null) {
                mEmergencyDscHolder = new DomainSelectionConnectionHolder(emergency);
            }
            return mEmergencyDscHolder;
        } else {
                mDscHolder = holder;
            if (mDscHolder == null) {
                mDscHolder = new DomainSelectionConnectionHolder(emergency);
            }
            return mDscHolder;
        }

        holder.setConnection(connection);

        return holder;
    }

    /**
@@ -1102,6 +1067,8 @@ public class SmsDispatchersController extends Handler {
     *
     * @param holder The {@link DomainSelectionConnectionHolder} that contains the
     *               {@link DomainSelectionConnection} and its related information.
     * @param request The {@link PendingRequest} that stores the SMS request
     *                (data, text, multipart text) to be sent.
     * @param logTag The log string.
     */
    private void requestDomainSelection(@NonNull DomainSelectionConnectionHolder holder,
@@ -1111,6 +1078,21 @@ public class SmsDispatchersController extends Handler {
        // the domain selection by adding this request to the pending list.
        holder.addRequest(request);

        if (holder.getConnection() == null) {
            DomainSelectionConnection connection =
                    mDomainSelectionResolverProxy.getDomainSelectionConnection(
                            mPhone, DomainSelectionService.SELECTOR_TYPE_SMS, holder.isEmergency());
            if (connection == null) {
                logd("requestDomainSelection: fallback for " + logTag);
                // If the domain selection connection is not available,
                // fallback to the legacy implementation.
                sendAllPendingRequests(holder, NetworkRegistrationInfo.DOMAIN_UNKNOWN);
                return;
            } else {
                holder.setConnection(connection);
            }
        }

        if (!isDomainSelectionRequested) {
            if (VDBG) {
                logd("requestDomainSelection: " + logTag);
@@ -1571,12 +1553,7 @@ public class SmsDispatchersController extends Handler {
        }

        if (isSmsDomainSelectionEnabled()) {
            DomainSelectionConnectionHolder holder = getDomainSelectionConnection(false);

            // If the DomainSelectionConnection is not available,
            // fallback to the legacy implementation.
            if (holder != null && holder.getConnection() != null) {
                sendSmsUsingDomainSelection(holder,
            sendSmsUsingDomainSelection(getDomainSelectionConnectionHolder(false),
                    new PendingRequest(PendingRequest.TYPE_DATA, null, callingPackage,
                            destAddr, scAddr, asArrayList(sentIntent),
                            asArrayList(deliveryIntent), isForVvm, data, destPort, null, null,
@@ -1584,7 +1561,6 @@ public class SmsDispatchersController extends Handler {
                    "sendData");
            return;
        }
        }

        if (mImsSmsDispatcher.isAvailable()) {
            mImsSmsDispatcher.sendData(callingPackage, destAddr, scAddr, destPort, data, sentIntent,
@@ -1812,12 +1788,7 @@ public class SmsDispatchersController extends Handler {
        if (isSmsDomainSelectionEnabled()) {
            TelephonyManager tm = mContext.getSystemService(TelephonyManager.class);
            boolean isEmergency = tm.isEmergencyNumber(destAddr);
            DomainSelectionConnectionHolder holder = getDomainSelectionConnection(isEmergency);

            // If the DomainSelectionConnection is not available,
            // fallback to the legacy implementation.
            if (holder != null && holder.getConnection() != null) {
                sendSmsUsingDomainSelection(holder,
            sendSmsUsingDomainSelection(getDomainSelectionConnectionHolder(isEmergency),
                    new PendingRequest(PendingRequest.TYPE_TEXT, null, callingPkg,
                            destAddr, scAddr, asArrayList(sentIntent),
                            asArrayList(deliveryIntent), isForVvm, null, 0, asArrayList(text),
@@ -1826,7 +1797,6 @@ public class SmsDispatchersController extends Handler {
                    "sendText");
            return;
        }
        }

        if (mImsSmsDispatcher.isAvailable() || mImsSmsDispatcher.isEmergencySmsSupport(destAddr)) {
            mImsSmsDispatcher.sendText(destAddr, scAddr, text, sentIntent, deliveryIntent,
@@ -1961,12 +1931,7 @@ public class SmsDispatchersController extends Handler {
        if (isSmsDomainSelectionEnabled()) {
            TelephonyManager tm = mContext.getSystemService(TelephonyManager.class);
            boolean isEmergency = tm.isEmergencyNumber(destAddr);
            DomainSelectionConnectionHolder holder = getDomainSelectionConnection(isEmergency);

            // If the DomainSelectionConnection is not available,
            // fallback to the legacy implementation.
            if (holder != null && holder.getConnection() != null) {
                sendSmsUsingDomainSelection(holder,
            sendSmsUsingDomainSelection(getDomainSelectionConnectionHolder(isEmergency),
                    new PendingRequest(PendingRequest.TYPE_MULTIPART_TEXT, null,
                            callingPkg, destAddr, scAddr, sentIntents, deliveryIntents, false,
                            null, 0, parts, messageUri, persistMessage, priority, expectMore,
@@ -1974,7 +1939,6 @@ public class SmsDispatchersController extends Handler {
                    "sendMultipartText");
            return;
        }
        }

        if (mImsSmsDispatcher.isAvailable()) {
            mImsSmsDispatcher.sendMultipartText(destAddr, scAddr, parts, sentIntents,
+224 −2
Original line number Diff line number Diff line
@@ -816,15 +816,237 @@ public class SmsDispatchersControllerTest extends TelephonyTest {
        setUpDomainSelectionConnection();
        setUpSmsDispatchers();
        when(mFeatureFlags.smsDomainSelectionEnabled()).thenReturn(false);
        when(mImsSmsDispatcher.isAvailable()).thenReturn(true);

        mSmsDispatchersController.sendText("1111", "2222", "text", mSentIntent, null, null,
                "test-app", false, 0, false, 10, false, 1L, false);

        // Expect that the domain selection is not executed and
        // ImsSmsDispatcher handles this text directly.
        verify(mImsSmsDispatcher).sendText(eq("1111"), eq("2222"), eq("text"),
                eq(mSentIntent), any(), any(), eq("test-app"), eq(false), eq(0), eq(false), eq(10),
                eq(false), eq(1L), eq(false));
    }

    @Test
    @SmallTest
    public void testSendTextWhenDomainSelectionFinishedAndNewTextSent() throws Exception {
        setUpDomainSelectionConnection();
        setUpSmsDispatchers();

        mSmsDispatchersController.sendText("1111", "2222", "text", mSentIntent, null, null,
                "test-app", false, 0, false, 10, false, 1L, false);
        processAllMessages();

        // Expect that the domain selection logic will not be executed.
        SmsDispatchersController.DomainSelectionConnectionHolder holder =
                mSmsDispatchersController.testGetDomainSelectionConnectionHolder(false);
        assertNull(holder);
        verify(mSmsDsc).requestDomainSelection(any(), any());
        assertNotNull(holder);
        assertNotNull(holder.getConnection());
        assertTrue(holder.isDomainSelectionRequested());
        assertEquals(1, holder.getPendingRequests().size());

        // Expect that finishDomainSelection is called while a new pending request is posted.
        mDscFuture.complete(NetworkRegistrationInfo.DOMAIN_PS);

        SmsDomainSelectionConnection newSmsDsc = Mockito.mock(SmsDomainSelectionConnection.class);
        mSmsDispatchersController.setDomainSelectionResolverProxy(
                new SmsDispatchersController.DomainSelectionResolverProxy() {
                    @Override
                    @Nullable
                    public DomainSelectionConnection getDomainSelectionConnection(Phone phone,
                            @DomainSelectionService.SelectorType int selectorType,
                            boolean isEmergency) {
                        return newSmsDsc;
                    }

                    @Override
                    public boolean isDomainSelectionSupported() {
                        return true;
                    }
                });
        CompletableFuture newDscFuture = new CompletableFuture<>();
        when(newSmsDsc.requestDomainSelection(
                any(DomainSelectionService.SelectionAttributes.class),
                any(DomainSelectionConnection.DomainSelectionConnectionCallback.class)))
                .thenReturn(newDscFuture);

        // Expect that new domain selection connection is created and domain selection is performed.
        mSmsDispatchersController.sendText("1111", "2222", "text", mSentIntent, null, null,
                "test-app", false, 0, false, 10, false, 1L, false);
        processAllMessages();

        verify(mSmsDsc).finishSelection();

        verify(newSmsDsc).requestDomainSelection(any(), any());
        assertNotNull(holder.getConnection());
        assertTrue(holder.isDomainSelectionRequested());
        assertEquals(1, holder.getPendingRequests().size());

        newDscFuture.complete(NetworkRegistrationInfo.DOMAIN_PS);
        processAllMessages();

        verify(newSmsDsc).finishSelection();
        verify(mImsSmsDispatcher, times(2)).sendText(eq("1111"), eq("2222"), eq("text"),
                eq(mSentIntent), any(), any(), eq("test-app"), eq(false), eq(0), eq(false), eq(10),
                eq(false), eq(1L), eq(false));
        assertNull(holder.getConnection());
        assertFalse(holder.isDomainSelectionRequested());
        assertEquals(0, holder.getPendingRequests().size());
    }

    @Test
    @SmallTest
    public void testSendTextForEmergencyWhenDomainSelectionFinishedAndNewTextSent()
            throws Exception {
        setUpDomainSelectionConnection();
        setUpSmsDispatchers();
        setUpEmergencyStateTracker(DisconnectCause.NOT_DISCONNECTED);

        mSmsDispatchersController.sendText("911", "2222", "text", mSentIntent, null, null,
                "test-app", false, 0, false, 10, false, 1L, false);
        processAllMessages();

        SmsDispatchersController.DomainSelectionConnectionHolder holder =
                mSmsDispatchersController.testGetDomainSelectionConnectionHolder(true);
        verify(mEmergencySmsDsc).requestDomainSelection(any(), any());
        assertNotNull(holder);
        assertNotNull(holder.getConnection());
        assertTrue(holder.isEmergency());
        assertTrue(holder.isDomainSelectionRequested());
        assertEquals(1, holder.getPendingRequests().size());

        // Expect that finishDomainSelection is called while a new pending request is posted.
        mDscFuture.complete(NetworkRegistrationInfo.DOMAIN_PS);

        EmergencySmsDomainSelectionConnection newEmergencySmsDsc =
                Mockito.mock(EmergencySmsDomainSelectionConnection.class);
        mSmsDispatchersController.setDomainSelectionResolverProxy(
                new SmsDispatchersController.DomainSelectionResolverProxy() {
                    @Override
                    @Nullable
                    public DomainSelectionConnection getDomainSelectionConnection(Phone phone,
                            @DomainSelectionService.SelectorType int selectorType,
                            boolean isEmergency) {
                        return newEmergencySmsDsc;
                    }

                    @Override
                    public boolean isDomainSelectionSupported() {
                        return true;
                    }
                });
        CompletableFuture newDscFuture = new CompletableFuture<>();
        when(newEmergencySmsDsc.requestDomainSelection(
                any(DomainSelectionService.SelectionAttributes.class),
                any(DomainSelectionConnection.DomainSelectionConnectionCallback.class)))
                .thenReturn(newDscFuture);

        // Expect that new domain selection connection is created and domain selection is performed.
        mSmsDispatchersController.sendText("911", "2222", "text", mSentIntent, null, null,
                "test-app", false, 0, false, 10, false, 1L, false);
        processAllMessages();

        verify(mEmergencySmsDsc).finishSelection();

        verify(newEmergencySmsDsc).requestDomainSelection(any(), any());
        assertNotNull(holder.getConnection());
        assertTrue(holder.isDomainSelectionRequested());
        assertEquals(1, holder.getPendingRequests().size());

        newDscFuture.complete(NetworkRegistrationInfo.DOMAIN_PS);
        processAllMessages();

        verify(newEmergencySmsDsc).finishSelection();
        verify(mImsSmsDispatcher, times(2)).sendText(eq("911"), eq("2222"), eq("text"),
                eq(mSentIntent), any(), any(), eq("test-app"), eq(false), eq(0), eq(false), eq(10),
                eq(false), eq(1L), eq(false));
        assertNull(holder.getConnection());
        assertFalse(holder.isDomainSelectionRequested());
        assertEquals(0, holder.getPendingRequests().size());
    }

    @Test
    @SmallTest
    public void testSendTextFallbackWhenDomainSelectionConnectionNotCreated() throws Exception {
        mSmsDispatchersController.setDomainSelectionResolverProxy(
                new SmsDispatchersController.DomainSelectionResolverProxy() {
                    @Override
                    @Nullable
                    public DomainSelectionConnection getDomainSelectionConnection(Phone phone,
                            @DomainSelectionService.SelectorType int selectorType,
                            boolean isEmergency) {
                        return null;
                    }

                    @Override
                    public boolean isDomainSelectionSupported() {
                        return true;
                    }
                });
        when(mFeatureFlags.smsDomainSelectionEnabled()).thenReturn(true);
        setUpSmsDispatchers();
        when(mImsSmsDispatcher.isAvailable()).thenReturn(true);

        // Expect that creating a domain selection connection is failed and
        // fallback to the legacy implementation.
        mSmsDispatchersController.sendText("1111", "2222", "text", mSentIntent, null, null,
                "test-app", false, 0, false, 10, false, 1L, false);
        processAllMessages();

        SmsDispatchersController.DomainSelectionConnectionHolder holder =
                mSmsDispatchersController.testGetDomainSelectionConnectionHolder(false);
        assertNotNull(holder);
        assertNull(holder.getConnection());
        assertFalse(holder.isDomainSelectionRequested());
        assertEquals(0, holder.getPendingRequests().size());

        verify(mImsSmsDispatcher).sendText(eq("1111"), eq("2222"), eq("text"), eq(mSentIntent),
                any(), any(), eq("test-app"), eq(false), eq(0), eq(false), eq(10), eq(false),
                eq(1L), eq(false));
    }

    @Test
    @SmallTest
    public void testSendTextFallbackForEmergencyWhenDomainSelectionConnectionNotCreated()
            throws Exception {
        mSmsDispatchersController.setDomainSelectionResolverProxy(
                new SmsDispatchersController.DomainSelectionResolverProxy() {
                    @Override
                    @Nullable
                    public DomainSelectionConnection getDomainSelectionConnection(Phone phone,
                            @DomainSelectionService.SelectorType int selectorType,
                            boolean isEmergency) {
                        return null;
                    }

                    @Override
                    public boolean isDomainSelectionSupported() {
                        return true;
                    }
                });
        when(mFeatureFlags.smsDomainSelectionEnabled()).thenReturn(true);
        setUpSmsDispatchers();
        setUpEmergencyStateTracker(DisconnectCause.NOT_DISCONNECTED);
        when(mImsSmsDispatcher.isAvailable()).thenReturn(true);

        // Expect that creating a domain selection connection is failed and
        // fallback to the legacy implementation.
        mSmsDispatchersController.sendText("911", "2222", "text", mSentIntent, null, null,
                "test-app", false, 0, false, 10, false, 1L, false);
        processAllMessages();

        SmsDispatchersController.DomainSelectionConnectionHolder holder =
                mSmsDispatchersController.testGetDomainSelectionConnectionHolder(true);
        assertNotNull(holder);
        assertNull(holder.getConnection());
        assertTrue(holder.isEmergency());
        assertFalse(holder.isDomainSelectionRequested());
        assertEquals(0, holder.getPendingRequests().size());

        verify(mImsSmsDispatcher).sendText(eq("911"), eq("2222"), eq("text"), eq(mSentIntent),
                any(), any(), eq("test-app"), eq(false), eq(0), eq(false), eq(10), eq(false),
                eq(1L), eq(false));
    }

    private void switchImsSmsFormat(int phoneType) {