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

Commit 35b1701c authored by Jack Yu's avatar Jack Yu
Browse files

Fixed race condition when network request removed

When data network enters disconnected state, a
network request might be removed without setting
the state back to unstatisfied. Fixed by removing
the incorrect logic in detachNetworkRequest. Also
rearranged the callback order to make sure all
network requests are detached before calling the
disconnected callback.

Test: atest DataNetworkControllerTest
Fix: 227280207

Change-Id: Ie69d8a49f8909988367565efa5a482481c8dc6b6
parent 1cbd74a9
Loading
Loading
Loading
Loading
+20 −9
Original line number Diff line number Diff line
@@ -999,6 +999,7 @@ public class DataNetwork extends StateMachine {
                        networkRequest.setState(TelephonyNetworkRequest.REQUEST_STATE_UNSATISFIED);
                        networkRequest.setAttachedNetwork(null);
                    }
                    log("All network requests detached.");
                    mAttachedNetworkRequestList.clear();
                    break;
                }
@@ -1400,6 +1401,24 @@ public class DataNetwork extends StateMachine {
        @Override
        public void enter() {
            logl("Data network disconnected. mEverConnected=" + mEverConnected);
            // Preserve the list for onSetupDataFailed callback, because we need to pass that list
            // back to DataNetworkController, but after EVENT_DETACH_ALL_NETWORK_REQUESTS gets
            // processed, the network request list would become empty.
            NetworkRequestList requestList = new NetworkRequestList(mAttachedNetworkRequestList);

            // The detach all network requests must be the last message to handle.
            sendMessage(EVENT_DETACH_ALL_NETWORK_REQUESTS);
            // Gracefully handle all the un-processed events then quit the state machine.
            // quit() throws a QUIT event to the end of message queue. All the events before quit()
            // will be processed. Events after quit() will not be processed.
            quit();

            //************************************************************//
            // DO NOT POST ANY EVENTS AFTER HERE.                         //
            // THE STATE MACHINE WONT PROCESS EVENTS AFTER QUIT.          //
            // ONLY CLEANUP SHOULD BE PERFORMED AFTER THIS.               //
            //************************************************************//

            if (mEverConnected) {
                mDataNetworkCallback.invokeFromExecutor(() -> mDataNetworkCallback
                        .onDisconnected(DataNetwork.this, mFailCause));
@@ -1409,15 +1428,8 @@ public class DataNetwork extends StateMachine {
            } else {
                mDataNetworkCallback.invokeFromExecutor(() -> mDataNetworkCallback
                        .onSetupDataFailed(DataNetwork.this,
                                new NetworkRequestList(mAttachedNetworkRequestList),
                                mFailCause, mRetryDelayMillis));
                                requestList, mFailCause, mRetryDelayMillis));
            }
            // The detach all network requests must be the last message to handle.
            sendMessage(EVENT_DETACH_ALL_NETWORK_REQUESTS);
            // Gracefully handle all the un-processed events then quit the state machine.
            // quit() throws a QUIT event to the end of message queue. All the events before quit()
            // will be processed. Events after quit() will not be processed.
            quit();
            notifyPreciseDataConnectionState();
            mNetworkAgent.unregister();
            mDataCallSessionStats.onDataCallDisconnected(mFailCause);
@@ -1525,7 +1537,6 @@ public class DataNetwork extends StateMachine {
     */
    public void detachNetworkRequest(@NonNull TelephonyNetworkRequest networkRequest) {
        if (getCurrentState() == null || isDisconnected()) {
            mAttachedNetworkRequestList.remove(networkRequest);
            return;
        }
        sendMessage(obtainMessage(EVENT_DETACH_NETWORK_REQUEST, networkRequest));
+1 −0
Original line number Diff line number Diff line
@@ -771,6 +771,7 @@ public abstract class TelephonyTest {
        Settings.Global.putInt(resolver, Settings.Global.DEVICE_PROVISIONED, 1);
        Settings.Global.putInt(resolver,
                Settings.Global.DEVICE_PROVISIONING_MOBILE_DATA_ENABLED, 1);
        Settings.Global.putInt(resolver, Settings.Global.DATA_ROAMING, 0);
        doReturn(mDataThrottler).when(mDcTracker).getDataThrottler();
        doReturn(-1L).when(mDataThrottler).getRetryTime(anyInt());