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

Commit 65613985 authored by Neil Fuller's avatar Neil Fuller
Browse files

Prevent undesirable scheduling behavior 

Avoid a case discovered by inspection.

Before this commit:

If the most recent time result is too old, every call to
refreshIfRequiredAndReschedule() will result in a refresh attempt until
one is successful. These calls can happen for reasons besides a
scheduled refresh, e.g. if the network connectivity changes. These
shouldn't happen often, but it not good to allow unrestricted server
load if the servers are down or unreachable.

After this commit:

This commit adds a minimum time between refresh attempts. This means
that refreshIfRequiredAndReschedule() may not result in a refresh. The
next scheduled refresh is still based on when the last refresh was
attempted.  This commit adds a test to demonstrate the new behavior.

All existing tests are unaffected.

Bug: 222295093
Test: atest services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java
Change-Id: Ife0c0b4826999525a8937e35168723e9b44c42dc
parent 469925d2

services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java

+71 −10
Original line number Diff line number Diff line
@@ -371,6 +371,10 @@ public class NetworkTimeUpdateService extends Binder { 
         * A shortened interval between refresh attempts used after a failure to refresh.

         * Always shorter than {@link #mNormalPollingIntervalMillis} and only used when {@link

         * #mTryAgainTimesMax} != 0.

         *

         * <p>This value is also the lower bound for the interval allowed between successive

         * refreshes when the latest time result is missing or too old, e.g. a refresh may not be

         * triggered when network connectivity is restored if the last attempt was too recent.

         */

        private final int mShortPollingIntervalMillis;
@@ -384,6 +388,21 @@ public class NetworkTimeUpdateService extends Binder { 


        private final NtpTrustedTime mNtpTrustedTime;



        /**

         * Records the time of the last refresh attempt (successful or otherwise) by this service.

         * This is used when scheduling the next refresh attempt. In cases where {@link

         * #refreshIfRequiredAndReschedule} is called too frequently, this will prevent each call

         * resulting in a network request. See also {@link #mShortPollingIntervalMillis}.

         *

         * <p>Time servers are a shared resource and so Android should avoid loading them.

         * Generally, a refresh attempt will succeed and the service won't need to make further

         * requests and this field will not limit requests.

         */

        // This field is only updated and accessed by the mHandler thread (except dump()).

        @GuardedBy("this")

        @ElapsedRealtimeLong

        private Long mLastRefreshAttemptElapsedRealtimeMillis;



        /**

         * Keeps track of successive time refresh failures have occurred. This is reset to zero when

         * time refresh is successful or if the number exceeds (a non-negative) {@link
@@ -413,7 +432,7 @@ public class NetworkTimeUpdateService extends Binder { 
        @Override

        public boolean forceRefreshForTests(

                @NonNull Network network, @NonNull RefreshCallbacks refreshCallbacks) {

            boolean refreshSuccessful = mNtpTrustedTime.forceRefresh(network);

            boolean refreshSuccessful = tryRefresh(network);

            logToDebugAndDumpsys("forceRefreshForTests: refreshSuccessful=" + refreshSuccessful);



            if (refreshSuccessful) {
@@ -437,14 +456,16 @@ public class NetworkTimeUpdateService extends Binder { 
                // calculateTimeResultAgeMillis() safely handles a null initialTimeResult.

                long timeResultAgeMillis = calculateTimeResultAgeMillis(

                        initialTimeResult, currentElapsedRealtimeMillis);

                shouldAttemptRefresh = timeResultAgeMillis >= mNormalPollingIntervalMillis;

                shouldAttemptRefresh =

                        timeResultAgeMillis >= mNormalPollingIntervalMillis

                        && isRefreshAllowed(currentElapsedRealtimeMillis);

            }



            boolean refreshSuccessful = false;

            if (shouldAttemptRefresh) {

                // This is a blocking call. Deliberately invoked without holding the "this" monitor

                // to avoid blocking logic that wants to use the "this" monitor.

                refreshSuccessful = mNtpTrustedTime.forceRefresh(network);

                refreshSuccessful = tryRefresh(network);

            }



            synchronized (this) {
@@ -495,16 +516,32 @@ public class NetworkTimeUpdateService extends Binder { 


                // (Re)schedule the next refresh based on the latest state.

                // Determine which refresh delay to use by using the current value of

                // mTryAgainCounter.

                // mTryAgainCounter. The refresh delay is applied to a different point in time

                // depending on whether the latest available time result (if any) is still

                // considered fresh to ensure the delay acts correctly.

                long refreshDelayMillis = mTryAgainCounter > 0

                        ? mShortPollingIntervalMillis : mNormalPollingIntervalMillis;

                // Adjust the next refresh time for the age of the latest time result.

                if (latestTimeResultAgeMillis < refreshDelayMillis) {

                    refreshDelayMillis -= latestTimeResultAgeMillis;

                }



                long nextRefreshElapsedRealtimeMillis =

                long nextRefreshElapsedRealtimeMillis;

                if (latestTimeResultAgeMillis < mNormalPollingIntervalMillis) {

                    // The latest time result is fresh, use it to determine when next to refresh.

                    nextRefreshElapsedRealtimeMillis =

                            latestTimeResult.getElapsedRealtimeMillis() + refreshDelayMillis;

                } else if (mLastRefreshAttemptElapsedRealtimeMillis != null) {

                    // The latest time result is missing or old and still needs to be refreshed.

                    // mLastRefreshAttemptElapsedRealtimeMillis, which should always be set by this

                    // point because there's no fresh time result, should be very close to

                    // currentElapsedRealtimeMillis unless the refresh was not allowed.

                    nextRefreshElapsedRealtimeMillis =

                            mLastRefreshAttemptElapsedRealtimeMillis + refreshDelayMillis;

                } else {

                    // This should not happen: mLastRefreshAttemptElapsedRealtimeMillis should

                    // always be non-null by this point.

                    logToDebugAndDumpsys(

                            "mLastRefreshAttemptElapsedRealtimeMillis unexpectedly missing."

                                    + " Scheduling using currentElapsedRealtimeMillis");

                    nextRefreshElapsedRealtimeMillis =

                            currentElapsedRealtimeMillis + refreshDelayMillis;

                }

                refreshCallbacks.scheduleNextRefresh(nextRefreshElapsedRealtimeMillis);



                logToDebugAndDumpsys("refreshIfRequiredAndReschedule:"
@@ -535,6 +572,26 @@ public class NetworkTimeUpdateService extends Binder { 
                    : timeResult.getAgeMillis(currentElapsedRealtimeMillis);

        }



        @GuardedBy("this")

        private boolean isRefreshAllowed(@ElapsedRealtimeLong long currentElapsedRealtimeMillis) {

            if (mLastRefreshAttemptElapsedRealtimeMillis == null) {

                return true;

            }

            // Use the second meaning of mShortPollingIntervalMillis: to determine the minimum time

            // allowed after an unsuccessful refresh before another can be attempted.

            long nextRefreshAllowedElapsedRealtimeMillis =

                    mLastRefreshAttemptElapsedRealtimeMillis + mShortPollingIntervalMillis;

            return currentElapsedRealtimeMillis >= nextRefreshAllowedElapsedRealtimeMillis;

        }



        private boolean tryRefresh(@NonNull Network network) {

            long currentElapsedRealtimeMillis = mElapsedRealtimeMillisSupplier.get();

            synchronized (this) {

                mLastRefreshAttemptElapsedRealtimeMillis = currentElapsedRealtimeMillis;

            }

            return mNtpTrustedTime.forceRefresh(network);

        }



        /** Suggests the time to the time detector. It may choose use it to set the system clock. */

        private void makeNetworkTimeSuggestion(@NonNull TimeResult ntpResult,

                @NonNull String debugInfo, @NonNull RefreshCallbacks refreshCallbacks) {
@@ -555,6 +612,10 @@ public class NetworkTimeUpdateService extends Binder { 
            ipw.println("mTryAgainTimesMax=" + mTryAgainTimesMax);



            synchronized (this) {

                String lastRefreshAttemptValue = mLastRefreshAttemptElapsedRealtimeMillis == null

                        ? "null"

                        : formatElapsedRealtimeMillis(mLastRefreshAttemptElapsedRealtimeMillis);

                ipw.println("mLastRefreshAttemptElapsedRealtimeMillis=" + lastRefreshAttemptValue);

                ipw.println("mTryAgainCounter=" + mTryAgainCounter);

            }

            ipw.println();

services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java

+83 −0
Original line number Diff line number Diff line
@@ -555,6 +555,89 @@ public class NetworkTimeUpdateServiceTest { 
        verify(mockCallback, never()).submitSuggestion(any());

    }



    /**

     * Confirms that if a refreshIfRequiredAndReschedule() call is made and there was a recently

     * failed refresh, then another won't be scheduled too soon.

     */

    @Test

    public void engineImpl_refreshIfRequiredAndReschedule_minimumRefreshTimeEnforced() {

        mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);



        int normalPollingIntervalMillis = 7777777;

        int shortPollingIntervalMillis = 3333;

        int tryAgainTimesMax = 0;

        NetworkTimeUpdateService.Engine engine = new NetworkTimeUpdateService.EngineImpl(

                mFakeElapsedRealtimeClock,

                normalPollingIntervalMillis, shortPollingIntervalMillis, tryAgainTimesMax,

                mMockNtpTrustedTime);



        NtpTrustedTime.TimeResult timeResult = createNtpTimeResult(

                mFakeElapsedRealtimeClock.getElapsedRealtimeMillis());



        // Simulate an initial call to refreshIfRequiredAndReschedule() prime the "last refresh

        // attempt" time. A cached time value is available, but it's too old but the refresh

        // attempt will fail.

        long lastRefreshAttemptElapsedMillis;

        {

            // Increment the clock, enough to consider the cached value too old.

            mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis);

            when(mMockNtpTrustedTime.getCachedTimeResult()).thenReturn(timeResult);

            when(mMockNtpTrustedTime.forceRefresh(mDummyNetwork)).thenReturn(false);



            RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);

            // Trigger the engine's logic.

            engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);



            // Expect a refresh attempt to have been made.

            verify(mMockNtpTrustedTime, times(1)).forceRefresh(mDummyNetwork);

            lastRefreshAttemptElapsedMillis = mFakeElapsedRealtimeClock.getElapsedRealtimeMillis();



            // The next wake-up should be rescheduled using the normalPollingIntervalMillis.

            // Because the time signal age > normalPollingIntervalMillis, the last refresh attempt

            // time will be used.

            long expectedDelayMillis = normalPollingIntervalMillis;

            long expectedNextRefreshElapsedMillis =

                    lastRefreshAttemptElapsedMillis + expectedDelayMillis;

            verify(mockCallback).scheduleNextRefresh(expectedNextRefreshElapsedMillis);



            // Suggestions should not be made if the cached time value is too old.

            verify(mockCallback, never()).submitSuggestion(any());



            reset(mMockNtpTrustedTime);

        }



        // Simulate a second call to refreshIfRequiredAndReschedule() very soon after the first, as

        // might happen if there were a network state change.

        // The cached time value is available, but it's still too old. Because the last call was so

        // recent, no refresh should take place and the next scheduled refresh time should be

        // set appropriately based on the last attempt.

        {

            // Increment the clock by a relatively small amount so that it's considered "too soon".

            mFakeElapsedRealtimeClock.incrementMillis(shortPollingIntervalMillis / 2);



            when(mMockNtpTrustedTime.getCachedTimeResult()).thenReturn(timeResult);



            RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);

            // Trigger the engine's logic.

            engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);



            // Expect no refresh attempt to have been made: time elapsed isn't enough.

            verify(mMockNtpTrustedTime, never()).forceRefresh(any());



            // The next wake-up should be rescheduled using the normal polling interval and the last

            // refresh attempt time.

            long expectedDelayMillis = normalPollingIntervalMillis;

            long expectedNextRefreshElapsedMillis =

                    lastRefreshAttemptElapsedMillis + expectedDelayMillis;

            verify(mockCallback).scheduleNextRefresh(expectedNextRefreshElapsedMillis);



            // Suggestions should not be made if the cached time value is too old.

            verify(mockCallback, never()).submitSuggestion(any());



            reset(mMockNtpTrustedTime);

        }

    }



    private static NetworkTimeSuggestion createExpectedSuggestion(

            NtpTrustedTime.TimeResult timeResult) {

        UnixEpochTime unixEpochTime = new UnixEpochTime(