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

Commit d9a5a874 authored by Alex Buynytskyy's avatar Alex Buynytskyy
Browse files

Reduce max wait time for package settings write.

Reported multiple crashes when we are waiting for 10+mins for a lock.
There is not reason to wait for this long, and the callstacks are not
helpful.
While it's not ideal to postpone writes, still, it's better than a
crash.

Bug: 319038955
Test: presubmit
Change-Id: I253bc9ffb9eb16747876d14a15edb74eda9207e3
parent c114177f
Loading
Loading
Loading
Loading
+11 −2
Original line number Diff line number Diff line
@@ -116,10 +116,19 @@ final class PackageHandler extends Handler {
                }
            } break;
            case WRITE_SETTINGS: {
                mPm.writeSettings(/*sync=*/false);
                if (!mPm.tryWriteSettings(/*sync=*/false)) {
                    // Failed to write.
                    this.removeMessages(WRITE_SETTINGS);
                    mPm.scheduleWriteSettings();
                }
            } break;
            case WRITE_PACKAGE_LIST: {
                mPm.writePackageList(msg.arg1);
                int userId = msg.arg1;
                if (!mPm.tryWritePackageList(userId)) {
                    // Failed to write.
                    this.removeMessages(WRITE_PACKAGE_LIST);
                    mPm.scheduleWritePackageList(userId);
                }
            } break;
            case CHECK_PENDING_VERIFICATION: {
                final int verificationId = msg.arg1;
+33 −9
Original line number Diff line number Diff line
@@ -489,6 +489,9 @@ public class PackageManagerService implements PackageSender, TestUtilityService
     */
    static final long WATCHDOG_TIMEOUT = 1000*60*10;     // ten minutes

    // How long to wait for Lock in async writeSettings and writePackageList.
    private static final long WRITE_LOCK_TIMEOUT_MS = 1000 * 10;   // 10 seconds

    /**
     * Default IncFs timeouts. Maximum values in IncFs is 1hr.
     *
@@ -1562,7 +1565,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService
        }
    }

    private void scheduleWritePackageListLocked(int userId) {
    void scheduleWritePackageList(int userId) {
        invalidatePackageInfoCache();
        if (!mHandler.hasMessages(WRITE_PACKAGE_LIST)) {
            Message msg = mHandler.obtainMessage(WRITE_PACKAGE_LIST);
@@ -1614,22 +1617,41 @@ public class PackageManagerService implements PackageSender, TestUtilityService
        mSettings.writePackageRestrictions(dirtyUsers);
    }

    void writeSettings(boolean sync) {
        synchronized (mLock) {
    private boolean tryUnderLock(boolean sync, long timeoutMs, Runnable runnable) {
        try {
            if (sync) {
                mLock.lock();
            } else if (!mLock.tryLock(timeoutMs, TimeUnit.MILLISECONDS)) {
                return false;
            }
            try {
                runnable.run();
                return true;
            } finally {
                mLock.unlock();
            }
        } catch (InterruptedException e) {
            Slog.e(TAG, "Failed to obtain mLock", e);
        }
        return false;
    }

    boolean tryWriteSettings(boolean sync) {
        return tryUnderLock(sync, WRITE_LOCK_TIMEOUT_MS, () -> {
            mHandler.removeMessages(WRITE_SETTINGS);
            mBackgroundHandler.removeMessages(WRITE_DIRTY_PACKAGE_RESTRICTIONS);
            writeSettingsLPrTEMP(sync);
            synchronized (mDirtyUsers) {
                mDirtyUsers.clear();
            }
        }
        });
    }

    void writePackageList(int userId) {
        synchronized (mLock) {
    boolean tryWritePackageList(int userId) {
        return tryUnderLock(/*sync=*/false, WRITE_LOCK_TIMEOUT_MS, () -> {
            mHandler.removeMessages(WRITE_PACKAGE_LIST);
            mSettings.writePackageListLPr(userId);
        }
        });
    }

    private static final Handler.Callback BACKGROUND_HANDLER_CALLBACK = new Handler.Callback() {
@@ -3054,7 +3076,9 @@ public class PackageManagerService implements PackageSender, TestUtilityService
            if (mHandler.hasMessages(WRITE_SETTINGS)
                    || mBackgroundHandler.hasMessages(WRITE_DIRTY_PACKAGE_RESTRICTIONS)
                    || mHandler.hasMessages(WRITE_PACKAGE_LIST)) {
                writeSettings(/*sync=*/true);
                while (!tryWriteSettings(/*sync=*/true)) {
                    Slog.wtf(TAG, "Failed to write settings on shutdown");
                }
            }
        }
    }
@@ -4361,7 +4385,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService
        }
        synchronized (mLock) {
            scheduleWritePackageRestrictions(userId);
            scheduleWritePackageListLocked(userId);
            scheduleWritePackageList(userId);
            mAppsFilter.onUserCreated(snapshotComputer(), userId);
        }
    }
+3 −1
Original line number Diff line number Diff line
@@ -16,9 +16,11 @@

package com.android.server.pm;

import java.util.concurrent.locks.ReentrantLock;

/**
 * This is a unique class that is used as the PackageManager lock.  It can be targeted for lock
 * injection, similar to {@link ActivityManagerGlobalLock}.
 */
public class PackageManagerTracedLock {
public class PackageManagerTracedLock extends ReentrantLock {
}