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

Commit 9c0bfbb2 authored by Lee Shombert's avatar Lee Shombert
Browse files

Correct JobServiceContext use of AnrTimer

This fixes two places in JobServiceContext that failed to correctly
cancel a timer.  The JobAnrTimer relies on an up-to-date value in
mRunningCallback: in two places, mRunningCallback was set to null
before the timer was canceled.  This change cancels the timer before
setting that attribute to null.

AnrTimer itself now detects calls into the object that occur after the
object has been closed.  Calling start() on a closed object is a
run-time error.  Calling cancel(), discard(), or accept() on a close
object is considered a permissible race condition and these calls are
silently ignored.

Tested with an instrumented build to verify that the JobServiceContext
no longer generates any invalid timeouts.

Flag: com.android.server.utils.anr_timer_for_job_service
Bug: 408440679
Test: atest
 * FrameworksMockingServicesTests_com_android_server_job
Change-Id: I3b3541ba287ab0b74da42571f467c61efd12b9ca
parent eb1109d0
Loading
Loading
Loading
Loading
+6 −2
Original line number Diff line number Diff line
@@ -498,6 +498,8 @@ public final class JobServiceContext implements ServiceConnection {
                    Slog.d(TAG, job.getServiceComponent().getShortClassName() + " unavailable.");
                }
                mContext.unbindService(this);
                // Ensure this occurs while mRunningCallback is still set.
                removeOpTimeOutLocked();
                mRunningJob = null;
                mRunningJobWorkType = WORK_TYPE_NONE;
                mRunningCallback = null;
@@ -505,7 +507,6 @@ public final class JobServiceContext implements ServiceConnection {
                mExecutionStartTimeElapsed = 0L;
                mWakeLock.release();
                mVerb = VERB_FINISHED;
                removeOpTimeOutLocked();
                return false;
            }
            mJobPackageTracker.noteActive(job);
@@ -1213,6 +1214,8 @@ public final class JobServiceContext implements ServiceConnection {
                            handleOpTimeoutLocked();
                        } else {
                            JobCallback jc = (JobCallback)message.obj;
                            // This is an unknown JobCallback.  It may, or may not, have an
                            // associated timer.
                            mAnrTimer.discard(jc);
                            StringBuilder sb = new StringBuilder(128);
                            sb.append("Ignoring timeout of no longer active job");
@@ -1765,6 +1768,8 @@ public final class JobServiceContext implements ServiceConnection {
        }
        final int workType = mRunningJobWorkType;
        mContext.unbindService(JobServiceContext.this);
        // Ensure this occurs while mRunningCallback is still set.
        removeOpTimeOutLocked();
        mWakeLock = null;
        mRunningJob = null;
        mRunningJobWorkType = WORK_TYPE_NONE;
@@ -1782,7 +1787,6 @@ public final class JobServiceContext implements ServiceConnection {
        mPendingInternalStopReason = 0;
        mPendingDebugStopReason = null;
        mPendingNetworkChange = null;
        removeOpTimeOutLocked();
        if (completedJob.isUserVisibleJob()) {
            mService.informObserversOfUserVisibleJobChange(this, completedJob, false);
        }
+8 −0
Original line number Diff line number Diff line
@@ -527,6 +527,8 @@ public class AnrTimer<V> implements AutoCloseable {
        @Override
        void start(@NonNull V arg, int pid, int uid, long timeoutMs) {
            synchronized (mLock) {
                if (mNative == 0) throw new IllegalStateException("timer service is closed");

                // If there is an existing timer, cancel it.  This is a nop if the timer does not
                // exist.
                if (cancel(arg)) mTotalRestarted++;
@@ -554,6 +556,8 @@ public class AnrTimer<V> implements AutoCloseable {
                if (timer == null) {
                    return false;
                }
                // Race conditions may lead to timer cancellation after the service was closed.
                if (mNative == 0) return false;
                if (!nativeAnrTimerCancel(mNative, timer)) {
                    // There may be an expiration message in flight.  Cancel it.
                    mHandler.removeMessages(mWhat, arg);
@@ -579,6 +583,8 @@ public class AnrTimer<V> implements AutoCloseable {
                    notFoundLocked("accept", arg);
                    return null;
                }
                // Race conditions may lead to timer acceptance after the service was closed.
                if (mNative == 0) return null;
                boolean accepted = nativeAnrTimerAccept(mNative, timer);
                trace("accept", timer);
                // If "accepted" is true then the native layer has pending operations against this
@@ -601,6 +607,8 @@ public class AnrTimer<V> implements AutoCloseable {
                    notFoundLocked("discard", arg);
                    return false;
                }
                // Race conditions may lead to timer discard  after the service was closed.
                if (mNative == 0) return false;
                nativeAnrTimerDiscard(mNative, timer);
                trace("discard", timer);
                return true;