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

Commit 9e92ae1c authored by Songchun Fan's avatar Songchun Fan
Browse files

[pm] revert lock changes to settings write 

This reverts ag/25983279, which introduced a behavior change in the
locking of writeSettings. As a result of that CL, the disk writes were
not guarded by "synchronized (mLock)" which may cause issues if the
settings are being modified at the same time as the writes.

BUG: 345278249
Test: presubmit
Change-Id: I3c855c986bb7bc9b7ab220e416305d562c621a5b
parent e55490cb

services/core/java/com/android/server/pm/PackageHandler.java

+2 −11
Original line number Diff line number Diff line
@@ -123,19 +123,10 @@ final class PackageHandler extends Handler { 
                }

            } break;

            case WRITE_SETTINGS: {

                if (!mPm.tryWriteSettings(/*sync=*/false)) {

                    // Failed to write.

                    this.removeMessages(WRITE_SETTINGS);

                    mPm.scheduleWriteSettings();

                }

                mPm.writeSettings(/*sync=*/false);

            } break;

            case WRITE_PACKAGE_LIST: {

                int userId = msg.arg1;

                if (!mPm.tryWritePackageList(userId)) {

                    // Failed to write.

                    this.removeMessages(WRITE_PACKAGE_LIST);

                    mPm.scheduleWritePackageList(userId);

                }

                mPm.writePackageList(msg.arg1);

            } break;

            case CHECK_PENDING_VERIFICATION: {

                final int verificationId = msg.arg1;

services/core/java/com/android/server/pm/PackageManagerService.java

+9 −34
Original line number Diff line number Diff line
@@ -485,9 +485,6 @@ 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.

     *
@@ -1576,7 +1573,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService 
        }

    }



    void scheduleWritePackageList(int userId) {

    void scheduleWritePackageListLocked(int userId) {

        invalidatePackageInfoCache();

        if (!mHandler.hasMessages(WRITE_PACKAGE_LIST)) {

            Message msg = mHandler.obtainMessage(WRITE_PACKAGE_LIST);
@@ -1628,42 +1625,22 @@ public class PackageManagerService implements PackageSender, TestUtilityService 
        mSettings.writePackageRestrictions(dirtyUsers);

    }



    private boolean tryUnderLock(boolean sync, long timeoutMs, Runnable runnable) {

        try {

            PackageManagerTracedLock.RawLock lock = mLock.getRawLock();

            if (sync) {

                lock.lock();

            } else if (!lock.tryLock(timeoutMs, TimeUnit.MILLISECONDS)) {

                return false;

            }

            try {

                runnable.run();

                return true;

            } finally {

                lock.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, () -> {

    void writeSettings(boolean sync) {

        synchronized (mLock) {

            mHandler.removeMessages(WRITE_SETTINGS);

            mBackgroundHandler.removeMessages(WRITE_DIRTY_PACKAGE_RESTRICTIONS);

            writeSettingsLPrTEMP(sync);

            synchronized (mDirtyUsers) {

                mDirtyUsers.clear();

            }

        });

        }

    }



    boolean tryWritePackageList(int userId) {

        return tryUnderLock(/*sync=*/false, WRITE_LOCK_TIMEOUT_MS, () -> {

    void writePackageList(int userId) {

        synchronized (mLock) {

            mHandler.removeMessages(WRITE_PACKAGE_LIST);

            mSettings.writePackageListLPr(userId);

        });

        }

    }



    private static final Handler.Callback BACKGROUND_HANDLER_CALLBACK = new Handler.Callback() {
@@ -3067,9 +3044,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService 
            if (mHandler.hasMessages(WRITE_SETTINGS)

                    || mBackgroundHandler.hasMessages(WRITE_DIRTY_PACKAGE_RESTRICTIONS)

                    || mHandler.hasMessages(WRITE_PACKAGE_LIST)) {

                while (!tryWriteSettings(/*sync=*/true)) {

                    Slog.wtf(TAG, "Failed to write settings on shutdown");

                }

                writeSettings(/*sync=*/true);

            }

        }

    }
@@ -4429,7 +4404,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService 
        }

        synchronized (mLock) {

            scheduleWritePackageRestrictions(userId);

            scheduleWritePackageList(userId);

            scheduleWritePackageListLocked(userId);

            mAppsFilter.onUserCreated(snapshotComputer(), userId);

        }

    }