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

Commit de91ac34 authored by Hai Zhang's avatar Hai Zhang
Browse files

Fix deadlock when using PackageManagerLocal while holding mLock.

The lock order requires obtaining mSnapshotLock before mLock, however
if a caller is called under mLock and tries to use
PackageManagerLocal, it will call snapshotComputer() which will try to
obtain mSnapshotLock and result in a deadlock.

However, actually mSnapshotLock is just a preliminary check that
exists as a less contended lock than mLock - it adds a third layer
between the regular double checked locking, but still always requires
holding the final mLock before actual mutations.

So if the caller is already holding mLock, we can simply let it
rebuild the snapshot directly when necessary. And in terms of the
triple checked locking, we just need to check whether sSnapshot has
become up-to-date again after obtaining mLock in the normal path.

Fixes: 263455681
Test: manual
Change-Id: I7d00ff2091331d02dc666f09cb62523679009300
parent 3690ef88
Loading
Loading
Loading
Loading
+16 −2
Original line number Diff line number Diff line
@@ -1105,8 +1105,9 @@ public class PackageManagerService implements PackageSender, TestUtilityService
    @Deprecated
    @NonNull
    public Computer snapshotComputer(boolean allowLiveComputer) {
        var isHoldingPackageLock = Thread.holdsLock(mLock);
        if (allowLiveComputer) {
            if (Thread.holdsLock(mLock)) {
            if (isHoldingPackageLock) {
                // If the current thread holds mLock then it may have modified state but not
                // yet invalidated the snapshot.  Always give the thread the live computer.
                return mLiveComputer;
@@ -1120,6 +1121,15 @@ public class PackageManagerService implements PackageSender, TestUtilityService
            return oldSnapshot.use();
        }

        if (isHoldingPackageLock) {
            // If the current thread holds mLock then it already has exclusive write access to the
            // two snapshot fields, and we can just go ahead and rebuild the snapshot.
            @SuppressWarnings("GuardedBy")
            var newSnapshot = rebuildSnapshot(oldSnapshot, pendingVersion);
            sSnapshot.set(newSnapshot);
            return newSnapshot.use();
        }

        synchronized (mSnapshotLock) {
            // Re-capture pending version in case a new invalidation occurred since last check
            var rebuildSnapshot = sSnapshot.get();
@@ -1137,7 +1147,11 @@ public class PackageManagerService implements PackageSender, TestUtilityService
                // Fetch version one last time to ensure that the rebuilt snapshot matches
                // the latest invalidation, which could have come in between entering the
                // SnapshotLock and mLock sync blocks.
                rebuildSnapshot = sSnapshot.get();
                rebuildVersion = sSnapshotPendingVersion.get();
                if (rebuildSnapshot != null && rebuildSnapshot.getVersion() == rebuildVersion) {
                    return rebuildSnapshot.use();
                }

                // Build the snapshot for this version
                var newSnapshot = rebuildSnapshot(rebuildSnapshot, rebuildVersion);
@@ -1147,7 +1161,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService
        }
    }

    @GuardedBy({ "mLock", "mSnapshotLock"})
    @GuardedBy("mLock")
    private Computer rebuildSnapshot(@Nullable Computer oldSnapshot, int newVersion) {
        var now = SystemClock.currentTimeMicro();
        var hits = oldSnapshot == null ? -1 : oldSnapshot.getUsed();