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

Commit 496ebf67 authored by Neil Fuller's avatar Neil Fuller
Browse files

Reduce field contention during dump()

Prevent a long-running forceRefresh() blocking dump and causing no logs
to be emitted.

Bug: 271975153
Test: Treehugger only
Change-Id: I223d9017bdbd7d9918a3b5a37a437dc539b2ce78
parent 3d84b3c1
Loading
Loading
Loading
Loading
+46 −23
Original line number Diff line number Diff line
@@ -216,19 +216,36 @@ public abstract class NtpTrustedTime implements TrustedTime {

    private static NtpTrustedTime sSingleton;

    /** A lock to prevent multiple refreshes taking place at the same time. */
    private final Object mRefreshLock = new Object();

    /** A lock to ensure safe read/writes to configuration. */
    private final Object mConfigLock = new Object();

    /** An in-memory config override for use during tests. */
    @GuardedBy("this")
    @GuardedBy("mConfigLock")
    @Nullable
    private NtpConfig mNtpConfigForTests;

    @GuardedBy("this")
    /**
     * The latest time result.
     *
     * <p>Written when holding {@link #mRefreshLock} but declared volatile and can be read outside
     * synchronized blocks to avoid blocking dump() during {@link #forceRefresh}.
     */
    @Nullable
    private URI mLastSuccessfulNtpServerUri;

    // Declared volatile and accessed outside synchronized blocks to avoid blocking reads during
    // forceRefresh().
    private volatile TimeResult mTimeResult;

    /**
     * The last successful NTP server URI, i.e. the one used to obtain {@link #mTimeResult} when it
     * is non-null.
     *
     * <p>Written when holding {@link #mRefreshLock} but declared volatile and can be read outside
     * synchronized blocks to avoid blocking dump() during {@link #forceRefresh}.
     */
    @Nullable
    private volatile URI mLastSuccessfulNtpServerUri;

    protected NtpTrustedTime() {
    }

@@ -246,7 +263,7 @@ public abstract class NtpTrustedTime implements TrustedTime {
     * test value, i.e. so the normal value will be used next time.
     */
    public void setServerConfigForTests(@NonNull NtpConfig ntpConfig) {
        synchronized (this) {
        synchronized (mConfigLock) {
            mNtpConfigForTests = ntpConfig;
        }
    }
@@ -254,7 +271,7 @@ public abstract class NtpTrustedTime implements TrustedTime {
    /** Forces a refresh using the default network. */
    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
    public boolean forceRefresh() {
        synchronized (this) {
        synchronized (mRefreshLock) {
            Network network = getDefaultNetwork();
            if (network == null) {
                if (LOGD) Log.d(TAG, "forceRefresh: no network available");
@@ -269,12 +286,13 @@ public abstract class NtpTrustedTime implements TrustedTime {
    public boolean forceRefresh(@NonNull Network network) {
        Objects.requireNonNull(network);

        synchronized (this) {
        synchronized (mRefreshLock) {
            // Prevent concurrent refreshes.
            return forceRefreshLocked(network);
        }
    }

    @GuardedBy("this")
    @GuardedBy("mRefreshLock")
    private boolean forceRefreshLocked(@NonNull Network network) {
        Objects.requireNonNull(network);

@@ -349,13 +367,14 @@ public abstract class NtpTrustedTime implements TrustedTime {
        return false;
    }

    @GuardedBy("this")
    private NtpConfig getNtpConfig() {
        synchronized (mConfigLock) {
            if (mNtpConfigForTests != null) {
                return mNtpConfigForTests;
            }
            return getNtpConfigInternal();
        }
    }

    /**
     * Returns the {@link NtpConfig} to use during an NTP query. This method can return {@code null}
@@ -363,6 +382,7 @@ public abstract class NtpTrustedTime implements TrustedTime {
     *
     * <p>This method has been made public for easy replacement during tests.
     */
    @GuardedBy("mConfigLock")
    @VisibleForTesting
    @Nullable
    public abstract NtpConfig getNtpConfigInternal();
@@ -479,14 +499,14 @@ public abstract class NtpTrustedTime implements TrustedTime {

    /** Sets the last received NTP time. Intended for use during tests. */
    public void setCachedTimeResult(TimeResult timeResult) {
        synchronized (this) {
        synchronized (mRefreshLock) {
            mTimeResult = timeResult;
        }
    }

    /** Clears the last received NTP time. Intended for use during tests. */
    public void clearCachedTimeResult() {
        synchronized (this) {
        synchronized (mRefreshLock) {
            mTimeResult = null;
        }
    }
@@ -585,15 +605,18 @@ public abstract class NtpTrustedTime implements TrustedTime {

    /** Prints debug information. */
    public void dump(PrintWriter pw) {
        synchronized (this) {
        synchronized (mConfigLock) {
            pw.println("getNtpConfig()=" + getNtpConfig());
            pw.println("mNtpConfigForTests=" + mNtpConfigForTests);
        }

        pw.println("mLastSuccessfulNtpServerUri=" + mLastSuccessfulNtpServerUri);
            pw.println("mTimeResult=" + mTimeResult);
            if (mTimeResult != null) {

        TimeResult timeResult = mTimeResult;
        pw.println("mTimeResult=" + timeResult);
        if (timeResult != null) {
            pw.println("mTimeResult.getAgeMillis()="
                        + Duration.ofMillis(mTimeResult.getAgeMillis()));
            }
                    + Duration.ofMillis(timeResult.getAgeMillis()));
        }
    }