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

Commit 92924ff7 authored by Amit Mahajan's avatar Amit Mahajan
Browse files

Fix race condition between SmsDispatcher and CarrierMessagingService 

The timeout message to track response was being posted after
response was already received from CarrierMessagingService in
some cases.

Also added more logging to debug SmsDispatcher issues.

Test: manually verified SMS sending

Bug: 190557106
Bug: 190851041
Bug: 190860074
Change-Id: Ie219cca053e64760a871592cb77db20b32214ed5
parent c66a8d63

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

+25 −12
Original line number Original line Diff line number Diff line
@@ -86,6 +86,7 @@ import com.android.telephony.Rlog; 
import java.io.FileDescriptor;

import java.io.FileDescriptor;

import java.io.PrintWriter;

import java.io.PrintWriter;

import java.util.ArrayList;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.HashMap;

import java.util.HashMap;

import java.util.List;

import java.util.List;

import java.util.Random;

import java.util.Random;
@@ -374,7 +375,7 @@ public abstract class SMSDispatcher extends Handler { 
        /**

        /**

         * Bind to carrierPackageName to send message through it

         * Bind to carrierPackageName to send message through it

         */

         */

        public void sendSmsByCarrierApp(String carrierPackageName,

        public synchronized void sendSmsByCarrierApp(String carrierPackageName,

                CarrierMessagingCallback senderCallback) {

                CarrierMessagingCallback senderCallback) {

            mCarrierPackageName = carrierPackageName;

            mCarrierPackageName = carrierPackageName;

            mSenderCallback = senderCallback;

            mSenderCallback = senderCallback;
@@ -390,7 +391,10 @@ public abstract class SMSDispatcher extends Handler { 
        }

        }





        /**

        /**

         * Callback received from mCarrierPackageName on binding to it is done

         * Callback received from mCarrierPackageName on binding to it is done.

         * NOTE: the implementations of this method must be synchronized to make sure it does not

         * get called before {@link #sendSmsByCarrierApp} completes and {@link #EVENT_TIMEOUT} is

         * posted

         */

         */

        public abstract void onServiceReady();

        public abstract void onServiceReady();
@@ -412,8 +416,8 @@ public abstract class SMSDispatcher extends Handler { 
        @Override

        @Override

        public void handleMessage(Message msg) {

        public void handleMessage(Message msg) {

            if (msg.what == EVENT_TIMEOUT) {

            if (msg.what == EVENT_TIMEOUT) {

                logWithLocalLog("handleMessage: did not receive response from "

                logWithLocalLog("handleMessage: No response from " + mCarrierPackageName

                        + mCarrierPackageName + " for " + mCarrierMessagingTimeout + " ms");

                        + " for " + mCarrierMessagingTimeout + " ms");

                AnomalyReporter.reportAnomaly(sAnomalyNoResponseFromCarrierMessagingService,

                AnomalyReporter.reportAnomaly(sAnomalyNoResponseFromCarrierMessagingService,

                        "No response from " + mCarrierPackageName);

                        "No response from " + mCarrierPackageName);

                onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

                onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);
@@ -443,7 +447,8 @@ public abstract class SMSDispatcher extends Handler { 
        }

        }





        @Override

        @Override

        public void onServiceReady() {

        public synchronized void onServiceReady() {

            Rlog.d(TAG, "TextSmsSender::onServiceReady");

            HashMap<String, Object> map = mTracker.getData();

            HashMap<String, Object> map = mTracker.getData();

            String text = (String) map.get(MAP_KEY_TEXT);

            String text = (String) map.get(MAP_KEY_TEXT);
@@ -459,10 +464,12 @@ public abstract class SMSDispatcher extends Handler { 
                            runnable -> runnable.run(),

                            runnable -> runnable.run(),

                            mSenderCallback);

                            mSenderCallback);

                } catch (RuntimeException e) {

                } catch (RuntimeException e) {

                    Rlog.e(TAG, "Exception sending the SMS: " + e.getMessage());

                    Rlog.e(TAG, "TextSmsSender::onServiceReady: Exception sending the SMS: "

                            + e.getMessage());

                    onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

                    onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

                }

                }

            } else {

            } else {

                Rlog.d(TAG, "TextSmsSender::onServiceReady: text == null");

                onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

                onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

            }

            }

        }

        }
@@ -495,7 +502,8 @@ public abstract class SMSDispatcher extends Handler { 
        }

        }





        @Override

        @Override

        public void onServiceReady() {

        public synchronized void onServiceReady() {

            Rlog.d(TAG, "DataSmsSender::onServiceReady");

            HashMap<String, Object> map = mTracker.getData();

            HashMap<String, Object> map = mTracker.getData();

            byte[] data = (byte[]) map.get(MAP_KEY_DATA);

            byte[] data = (byte[]) map.get(MAP_KEY_DATA);

            int destPort = (int) map.get(MAP_KEY_DEST_PORT);

            int destPort = (int) map.get(MAP_KEY_DEST_PORT);
@@ -513,11 +521,12 @@ public abstract class SMSDispatcher extends Handler { 
                            runnable -> runnable.run(),

                            runnable -> runnable.run(),

                            mSenderCallback);

                            mSenderCallback);

                } catch (RuntimeException e) {

                } catch (RuntimeException e) {

                    Rlog.e(TAG, "Exception sending the SMS: " + e

                    Rlog.e(TAG, "DataSmsSender::onServiceReady: Exception sending the SMS: " + e

                            + " " + SmsController.formatCrossStackMessageId(mTracker.mMessageId));

                            + " " + SmsController.formatCrossStackMessageId(mTracker.mMessageId));

                    onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

                    onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

                }

                }

            } else {

            } else {

                Rlog.d(TAG, "DataSmsSender::onServiceReady: data == null");

                onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

                onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

            }

            }

        }

        }
@@ -556,6 +565,7 @@ public abstract class SMSDispatcher extends Handler { 
         */

         */

        @Override

        @Override

        public void onSendSmsComplete(int result, int messageRef) {

        public void onSendSmsComplete(int result, int messageRef) {

            Rlog.d(TAG, "onSendSmsComplete: result=" + result + " messageRef=" + messageRef);

            if (mCallbackCalled) {

            if (mCallbackCalled) {

                logWithLocalLog("onSendSmsComplete: unexpected call");

                logWithLocalLog("onSendSmsComplete: unexpected call");

                AnomalyReporter.reportAnomaly(sAnomalyUnexpectedCallback,

                AnomalyReporter.reportAnomaly(sAnomalyUnexpectedCallback,
@@ -655,7 +665,8 @@ public abstract class SMSDispatcher extends Handler { 
        }

        }





        @Override

        @Override

        public void onServiceReady() {

        public synchronized void onServiceReady() {

            Rlog.d(TAG, "MultipartSmsSender::onServiceReady");

            boolean statusReportRequested = false;

            boolean statusReportRequested = false;

            for (SmsTracker tracker : mTrackers) {

            for (SmsTracker tracker : mTrackers) {

                if (tracker.mDeliveryIntent != null) {

                if (tracker.mDeliveryIntent != null) {
@@ -675,7 +686,7 @@ public abstract class SMSDispatcher extends Handler { 
                        runnable -> runnable.run(),

                        runnable -> runnable.run(),

                        mSenderCallback);

                        mSenderCallback);

            } catch (RuntimeException e) {

            } catch (RuntimeException e) {

                Rlog.e(TAG, "Exception sending the SMS: " + e);

                Rlog.e(TAG, "MultipartSmsSender::onServiceReady: Exception sending the SMS: " + e);

                onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

                onSendComplete(CarrierMessagingService.SEND_STATUS_RETRY_ON_CARRIER_NETWORK);

            }

            }

        }

        }
@@ -719,6 +730,8 @@ public abstract class SMSDispatcher extends Handler { 
         */

         */

        @Override

        @Override

        public void onSendMultipartSmsComplete(int result, int[] messageRefs) {

        public void onSendMultipartSmsComplete(int result, int[] messageRefs) {

            Rlog.d(TAG, "onSendMultipartSmsComplete: result=" + result + " messageRefs="

                    + Arrays.toString(messageRefs));

            if (mCallbackCalled) {

            if (mCallbackCalled) {

                logWithLocalLog("onSendMultipartSmsComplete: unexpected call");

                logWithLocalLog("onSendMultipartSmsComplete: unexpected call");

                AnomalyReporter.reportAnomaly(sAnomalyUnexpectedCallback,

                AnomalyReporter.reportAnomaly(sAnomalyUnexpectedCallback,
@@ -1274,7 +1287,7 @@ public abstract class SMSDispatcher extends Handler { 
    private boolean sendSmsByCarrierApp(boolean isDataSms, SmsTracker tracker ) {

    private boolean sendSmsByCarrierApp(boolean isDataSms, SmsTracker tracker ) {

        String carrierPackage = getCarrierAppPackageName();

        String carrierPackage = getCarrierAppPackageName();

        if (carrierPackage != null) {

        if (carrierPackage != null) {

            Rlog.d(TAG, "Found carrier package.");

            Rlog.d(TAG, "Found carrier package " + carrierPackage);

            SmsSender smsSender;

            SmsSender smsSender;

            if (isDataSms) {

            if (isDataSms) {

                smsSender = new DataSmsSender(tracker);

                smsSender = new DataSmsSender(tracker);
@@ -1485,7 +1498,7 @@ public abstract class SMSDispatcher extends Handler { 




        String carrierPackage = getCarrierAppPackageName();

        String carrierPackage = getCarrierAppPackageName();

        if (carrierPackage != null) {

        if (carrierPackage != null) {

            Rlog.d(TAG, "Found carrier package."

            Rlog.d(TAG, "Found carrier package " + carrierPackage

                    + " id: " + getMultiTrackermessageId(trackers));

                    + " id: " + getMultiTrackermessageId(trackers));

            MultipartSmsSender smsSender = new MultipartSmsSender(parts, trackers);

            MultipartSmsSender smsSender = new MultipartSmsSender(parts, trackers);

            smsSender.sendSmsByCarrierApp(carrierPackage,

            smsSender.sendSmsByCarrierApp(carrierPackage,