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

Commit 4d89e42e authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Watch all networks for job invalidation signals. 

Until now, ConnectivityController was only watching for the
system-wide "default" network to change.  Since the "default" network
can change on a per-UID basis, we could end up with jobs not being
rematched correctly.

To resolve this, we start watching for the firehose of all network
changes across the device.  Since this results in increased noise,
we add a local cache of per-UID answers, and only re-evaluate jobs
when the changing network is the "default" network for that UID.

For NetworkPolicyManager.Listener, we only need to listen to
onUidRulesChanged(), since the other events always boil down into
specific rule changes.

Extends unit tests to verify all this logic behaves correctly.

Test: atest com.android.server.job.controllers.ConnectivityControllerTest
Bug: 73553301
Change-Id: I5b23ecd1facb6de723c5a6a7ddf9769a032d439e
parent dac0fe91

services/core/java/com/android/server/job/JobSchedulerService.java

+5 −1
Original line number Diff line number Diff line
@@ -127,7 +127,7 @@ import java.util.function.Predicate; 
 * Any function with the suffix 'Locked' also needs to lock on {@link #mJobs}.

 * @hide

 */

public final class JobSchedulerService extends com.android.server.SystemService

public class JobSchedulerService extends com.android.server.SystemService

        implements StateChangedListener, JobCompletedListener {

    public static final String TAG = "JobScheduler";

    public static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);
@@ -781,6 +781,10 @@ public final class JobSchedulerService extends com.android.server.SystemService 
        }

    };



    public Context getTestableContext() {

        return getContext();

    }



    public Object getLock() {

        return mLock;

    }

services/core/java/com/android/server/job/controllers/ConnectivityController.java

+58 −49
Original line number Diff line number Diff line
@@ -30,12 +30,12 @@ import android.net.NetworkInfo; 
import android.net.NetworkPolicyManager;

import android.net.NetworkRequest;

import android.net.TrafficStats;

import android.os.Process;

import android.os.UserHandle;

import android.text.format.DateUtils;

import android.util.ArraySet;

import android.util.Log;

import android.util.Slog;

import android.util.SparseArray;

import android.util.proto.ProtoOutputStream;



import com.android.internal.annotations.GuardedBy;
@@ -46,6 +46,7 @@ import com.android.server.job.JobSchedulerService.Constants; 
import com.android.server.job.JobServiceContext;

import com.android.server.job.StateControllerProto;



import java.util.Objects;

import java.util.function.Predicate;



/**
@@ -63,7 +64,6 @@ public final class ConnectivityController extends StateController implements 


    private final ConnectivityManager mConnManager;

    private final NetworkPolicyManager mNetPolicyManager;

    private boolean mConnected;



    @GuardedBy("mLock")

    private final ArraySet<JobStatus> mTrackedJobs = new ArraySet<>();
@@ -74,9 +74,11 @@ public final class ConnectivityController extends StateController implements 
        mConnManager = mContext.getSystemService(ConnectivityManager.class);

        mNetPolicyManager = mContext.getSystemService(NetworkPolicyManager.class);



        mConnected = false;

        // We're interested in all network changes; internally we match these

        // network changes against the active network for each UID with jobs.

        final NetworkRequest request = new NetworkRequest.Builder().clearCapabilities().build();

        mConnManager.registerNetworkCallback(request, mNetworkCallback);



        mConnManager.registerDefaultNetworkCallback(mNetworkCallback);

        mNetPolicyManager.registerListener(mNetPolicyListener);

    }
@@ -198,14 +200,18 @@ public final class ConnectivityController extends StateController implements 
    }



    private boolean updateConstraintsSatisfied(JobStatus jobStatus) {

        final Network network = mConnManager.getActiveNetworkForUid(jobStatus.getSourceUid());

        final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(network);

        return updateConstraintsSatisfied(jobStatus, network, capabilities);

    }



    private boolean updateConstraintsSatisfied(JobStatus jobStatus, Network network,

            NetworkCapabilities capabilities) {

        // TODO: consider matching against non-active networks



        final int jobUid = jobStatus.getSourceUid();

        final boolean ignoreBlocked = (jobStatus.getFlags() & JobInfo.FLAG_WILL_BE_FOREGROUND) != 0;



        final Network network = mConnManager.getActiveNetworkForUid(jobUid, ignoreBlocked);

        final NetworkInfo info = mConnManager.getNetworkInfoForUid(network, jobUid, ignoreBlocked);

        final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(network);

        final NetworkInfo info = mConnManager.getNetworkInfoForUid(network,

                jobStatus.getSourceUid(), ignoreBlocked);



        final boolean connected = (info != null) && info.isConnected();

        final boolean satisfied = isSatisfied(jobStatus, network, capabilities, mConstants);
@@ -218,12 +224,6 @@ public final class ConnectivityController extends StateController implements 
        // using non-default routes.

        jobStatus.network = network;



        // Track system-uid connected/validated as a general reportable proxy for the

        // overall state of connectivity constraint satisfiability.

        if (jobUid == Process.SYSTEM_UID) {

            mConnected = connected;

        }



        if (DEBUG) {

            Slog.i(TAG, "Connectivity " + (changed ? "CHANGED" : "unchanged")

                    + " for " + jobStatus + ": connected=" + connected
@@ -233,18 +233,48 @@ public final class ConnectivityController extends StateController implements 
    }



    /**

     * Update all jobs tracked by this controller.

     * Update any jobs tracked by this controller that match given filters.

     *

     * @param uid only update jobs belonging to this UID, or {@code -1} to

     * @param filterUid only update jobs belonging to this UID, or {@code -1} to

     *            update all tracked jobs.

     * @param filterNetwork only update jobs that would use this

     *            {@link Network}, or {@code null} to update all tracked jobs.

     */

    private void updateTrackedJobs(int uid) {

    private void updateTrackedJobs(int filterUid, Network filterNetwork) {

        synchronized (mLock) {

            // Since this is a really hot codepath, temporarily cache any

            // answers that we get from ConnectivityManager.

            final SparseArray<Network> uidToNetwork = new SparseArray<>();

            final SparseArray<NetworkCapabilities> networkToCapabilities = new SparseArray<>();



            boolean changed = false;

            for (int i = mTrackedJobs.size() - 1; i >= 0; i--) {

                final JobStatus js = mTrackedJobs.valueAt(i);

                if (uid == -1 || uid == js.getSourceUid()) {

                    changed |= updateConstraintsSatisfied(js);

                final int uid = js.getSourceUid();



                final boolean uidMatch = (filterUid == -1 || filterUid == uid);

                if (uidMatch) {

                    Network network = uidToNetwork.get(uid);

                    if (network == null) {

                        network = mConnManager.getActiveNetworkForUid(uid);

                        uidToNetwork.put(uid, network);

                    }



                    // Update either when we have a network match, or when the

                    // job hasn't yet been evaluated against the currently

                    // active network; typically when we just lost a network.

                    final boolean networkMatch = (filterNetwork == null

                            || Objects.equals(filterNetwork, network));

                    final boolean forceUpdate = !Objects.equals(js.network, network);

                    if (networkMatch || forceUpdate) {

                        final int netId = network != null ? network.netId : -1;

                        NetworkCapabilities capabilities = networkToCapabilities.get(netId);

                        if (capabilities == null) {

                            capabilities = mConnManager.getNetworkCapabilities(network);

                            networkToCapabilities.put(netId, capabilities);

                        }

                        changed |= updateConstraintsSatisfied(js, network, capabilities);

                    }

                }

            }

            if (changed) {
@@ -273,19 +303,19 @@ public final class ConnectivityController extends StateController implements 


    private final NetworkCallback mNetworkCallback = new NetworkCallback() {

        @Override

        public void onCapabilitiesChanged(Network network, NetworkCapabilities networkCapabilities) {

        public void onCapabilitiesChanged(Network network, NetworkCapabilities capabilities) {

            if (DEBUG) {

                Slog.v(TAG, "onCapabilitiesChanged() : " + networkCapabilities);

                Slog.v(TAG, "onCapabilitiesChanged: " + network);

            }

            updateTrackedJobs(-1);

            updateTrackedJobs(-1, network);

        }



        @Override

        public void onLost(Network network) {

            if (DEBUG) {

                Slog.v(TAG, "Network lost");

                Slog.v(TAG, "onLost: " + network);

            }

            updateTrackedJobs(-1);

            updateTrackedJobs(-1, network);

        }

    };
@@ -293,25 +323,9 @@ public final class ConnectivityController extends StateController implements 
        @Override

        public void onUidRulesChanged(int uid, int uidRules) {

            if (DEBUG) {

                Slog.v(TAG, "Uid rules changed for " + uid);

                Slog.v(TAG, "onUidRulesChanged: " + uid);

            }

            updateTrackedJobs(uid);

        }



        @Override

        public void onRestrictBackgroundChanged(boolean restrictBackground) {

            if (DEBUG) {

                Slog.v(TAG, "Background restriction change to " + restrictBackground);

            }

            updateTrackedJobs(-1);

        }



        @Override

        public void onUidPoliciesChanged(int uid, int uidPolicies) {

            if (DEBUG) {

                Slog.v(TAG, "Uid policy changed for " + uid);

            }

            updateTrackedJobs(uid);

            updateTrackedJobs(uid, null);

        }

    };
@@ -319,9 +333,6 @@ public final class ConnectivityController extends StateController implements 
    @Override

    public void dumpControllerStateLocked(IndentingPrintWriter pw,

            Predicate<JobStatus> predicate) {

        pw.println("System connected: " + mConnected);

        pw.println();



        for (int i = 0; i < mTrackedJobs.size(); i++) {

            final JobStatus js = mTrackedJobs.valueAt(i);

            if (predicate.test(js)) {
@@ -343,8 +354,6 @@ public final class ConnectivityController extends StateController implements 
        final long token = proto.start(fieldId);

        final long mToken = proto.start(StateControllerProto.CONNECTIVITY);



        proto.write(StateControllerProto.ConnectivityController.IS_CONNECTED, mConnected);



        for (int i = 0; i < mTrackedJobs.size(); i++) {

            final JobStatus js = mTrackedJobs.valueAt(i);

            if (!predicate.test(js)) {

services/core/java/com/android/server/job/controllers/StateController.java

+1 −1
Original line number Diff line number Diff line
@@ -41,7 +41,7 @@ public abstract class StateController { 
    StateController(JobSchedulerService service) {

        mService = service;

        mStateChangedListener = service;

        mContext = service.getContext();

        mContext = service.getTestableContext();

        mLock = service.getLock();

        mConstants = service.getConstants();

    }

services/tests/servicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java

+145 −8
Original line number Diff line number Diff line
@@ -23,19 +23,28 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; 


import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.ArgumentMatchers.anyString;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.doNothing;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.reset;

import static org.mockito.Mockito.when;



import android.app.job.JobInfo;

import android.content.ComponentName;

import android.content.Context;

import android.content.pm.PackageManagerInternal;

import android.net.ConnectivityManager;

import android.net.ConnectivityManager.NetworkCallback;

import android.net.Network;

import android.net.NetworkCapabilities;

import android.net.NetworkInfo;

import android.net.NetworkInfo.DetailedState;

import android.net.NetworkPolicyManager;

import android.os.Build;

import android.os.Handler;

import android.os.SystemClock;

import android.support.test.runner.AndroidJUnit4;

import android.util.DataUnit;



import com.android.server.LocalServices;
@@ -45,14 +54,26 @@ import com.android.server.job.JobSchedulerService.Constants; 
import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.ArgumentCaptor;

import org.mockito.Mock;

import org.mockito.junit.MockitoJUnitRunner;



import java.time.Clock;

import java.time.ZoneOffset;



@RunWith(AndroidJUnit4.class)

@RunWith(MockitoJUnitRunner.class)

public class ConnectivityControllerTest {



    @Mock private Context mContext;

    @Mock private ConnectivityManager mConnManager;

    @Mock private NetworkPolicyManager mNetPolicyManager;

    @Mock private JobSchedulerService mService;



    private Constants mConstants;



    private static final int UID_RED = 10001;

    private static final int UID_BLUE = 10002;



    @Before

    public void setUp() throws Exception {

        // Assume all packages are current SDK
@@ -72,6 +93,19 @@ public class ConnectivityControllerTest { 


        // Assume default constants for now

        mConstants = new Constants();



        // Get our mocks ready

        when(mContext.getSystemServiceName(ConnectivityManager.class))

                .thenReturn(Context.CONNECTIVITY_SERVICE);

        when(mContext.getSystemService(Context.CONNECTIVITY_SERVICE))

                .thenReturn(mConnManager);

        when(mContext.getSystemServiceName(NetworkPolicyManager.class))

                .thenReturn(Context.NETWORK_POLICY_SERVICE);

        when(mContext.getSystemService(Context.NETWORK_POLICY_SERVICE))

                .thenReturn(mNetPolicyManager);

        when(mService.getTestableContext()).thenReturn(mContext);

        when(mService.getLock()).thenReturn(mService);

        when(mService.getConstants()).thenReturn(mConstants);

    }



    @Test
@@ -155,6 +189,99 @@ public class ConnectivityControllerTest { 
        }

    }



    @Test

    public void testUpdates() throws Exception {

        final ArgumentCaptor<NetworkCallback> callback = ArgumentCaptor

                .forClass(NetworkCallback.class);

        doNothing().when(mConnManager).registerNetworkCallback(any(), callback.capture());



        final ConnectivityController controller = new ConnectivityController(mService);



        final Network meteredNet = new Network(101);

        final NetworkCapabilities meteredCaps = createCapabilities();

        final Network unmeteredNet = new Network(202);

        final NetworkCapabilities unmeteredCaps = createCapabilities()

                .addCapability(NET_CAPABILITY_NOT_METERED);



        final JobStatus red = createJobStatus(createJob()

                .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), 0)

                .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED), UID_RED);

        final JobStatus blue = createJobStatus(createJob()

                .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), 0)

                .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY), UID_BLUE);



        // Pretend we're offline when job is added

        {

            reset(mConnManager);

            answerNetwork(UID_RED, null, null);

            answerNetwork(UID_BLUE, null, null);



            controller.maybeStartTrackingJobLocked(red, null);

            controller.maybeStartTrackingJobLocked(blue, null);



            assertFalse(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

            assertFalse(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

        }



        // Metered network

        {

            reset(mConnManager);

            answerNetwork(UID_RED, meteredNet, meteredCaps);

            answerNetwork(UID_BLUE, meteredNet, meteredCaps);



            callback.getValue().onCapabilitiesChanged(meteredNet, meteredCaps);



            assertFalse(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

            assertTrue(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

        }



        // Unmetered network background

        {

            reset(mConnManager);

            answerNetwork(UID_RED, meteredNet, meteredCaps);

            answerNetwork(UID_BLUE, meteredNet, meteredCaps);



            callback.getValue().onCapabilitiesChanged(unmeteredNet, unmeteredCaps);



            assertFalse(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

            assertTrue(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

        }



        // Lost metered network

        {

            reset(mConnManager);

            answerNetwork(UID_RED, unmeteredNet, unmeteredCaps);

            answerNetwork(UID_BLUE, unmeteredNet, unmeteredCaps);



            callback.getValue().onLost(meteredNet);



            assertTrue(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

            assertTrue(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

        }



        // Specific UID was blocked

        {

            reset(mConnManager);

            answerNetwork(UID_RED, null, null);

            answerNetwork(UID_BLUE, unmeteredNet, unmeteredCaps);



            callback.getValue().onCapabilitiesChanged(unmeteredNet, unmeteredCaps);



            assertFalse(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

            assertTrue(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY));

        }

    }



    private void answerNetwork(int uid, Network net, NetworkCapabilities caps) {

        when(mConnManager.getActiveNetworkForUid(eq(uid))).thenReturn(net);

        when(mConnManager.getNetworkCapabilities(eq(net))).thenReturn(caps);

        if (net != null) {

            final NetworkInfo ni = new NetworkInfo(ConnectivityManager.TYPE_WIFI, 0, null, null);

            ni.setDetailedState(DetailedState.CONNECTED, null, null);

            when(mConnManager.getNetworkInfoForUid(eq(net), eq(uid), anyBoolean())).thenReturn(ni);

        }

    }



    private static NetworkCapabilities createCapabilities() {

        return new NetworkCapabilities().addCapability(NET_CAPABILITY_INTERNET)

                .addCapability(NET_CAPABILITY_VALIDATED);
@@ -165,12 +292,22 @@ public class ConnectivityControllerTest { 
    }



    private static JobStatus createJobStatus(JobInfo.Builder job) {

        return createJobStatus(job, 0, Long.MAX_VALUE);

        return createJobStatus(job, android.os.Process.NOBODY_UID, 0, Long.MAX_VALUE);

    }



    private static JobStatus createJobStatus(JobInfo.Builder job, int uid) {

        return createJobStatus(job, uid, 0, Long.MAX_VALUE);

    }



    private static JobStatus createJobStatus(JobInfo.Builder job,

            long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis) {

        return createJobStatus(job, android.os.Process.NOBODY_UID,

                earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis);

    }



    private static JobStatus createJobStatus(JobInfo.Builder job, long earliestRunTimeElapsedMillis,

            long latestRunTimeElapsedMillis) {

        return new JobStatus(job.build(), 0, null, -1, 0, 0, null, earliestRunTimeElapsedMillis,

                latestRunTimeElapsedMillis, 0, 0, null, 0);

    private static JobStatus createJobStatus(JobInfo.Builder job, int uid,

            long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis) {

        return new JobStatus(job.build(), uid, null, -1, 0, 0, null,

                earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis, 0, 0, null, 0);

    }

}