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

Commit f3ce6028 authored by Yurii Zubrytskyi's avatar Yurii Zubrytskyi
Browse files

[res] Make IdmapDaemon more robust 

- Try to protect against the system shutdown when
  starting/stopping a service is impossible

- Fix the refcount for the service member in corner cases

- Don't access uninitialized connection object if its creation
  fails in the FRRO list function

- Get rid of the unneeded atomic int as it's always accessed
  under a lock

- Rename the start/stop service methods to explicitly point
  they're only called under a lock

Overall this makes the deadlocks much less probable, but not
gets rid of them completely

Fixes: 362083145
Fixes: 385464198
Test: build + boot
Flag: EXEMPT bugfix
Change-Id: Ib26f7317aec4dbb90616b630999e70ddcc5b884c
parent 556a10be

services/core/java/com/android/server/om/IdmapDaemon.java

+34 −22
Original line number Diff line number Diff line
@@ -40,7 +40,6 @@ import java.util.ArrayList; 
import java.util.Collections;

import java.util.List;

import java.util.concurrent.TimeoutException;

import java.util.concurrent.atomic.AtomicInteger;



/**

 * To prevent idmap2d from continuously running, the idmap daemon will terminate after 10 seconds
@@ -66,7 +65,7 @@ class IdmapDaemon { 


    private static IdmapDaemon sInstance;

    private volatile IIdmap2 mService;

    private final AtomicInteger mOpenedCount = new AtomicInteger();

    private int mOpenedCount = 0;

    private final Object mIdmapToken = new Object();



    /**
@@ -74,15 +73,20 @@ class IdmapDaemon { 
     * finalized, the idmap service will be stopped after a period of time unless another connection

     * to the service is open.

     **/

    private class Connection implements AutoCloseable {

    private final class Connection implements AutoCloseable {

        @Nullable

        private final IIdmap2 mIdmap2;

        private boolean mOpened = true;



        private Connection(IIdmap2 idmap2) {

            synchronized (mIdmapToken) {

                mOpenedCount.incrementAndGet();

        private Connection() {

            mIdmap2 = null;

            mOpened = false;

        }



        private Connection(@NonNull IIdmap2 idmap2) {

            mIdmap2 = idmap2;

            synchronized (mIdmapToken) {

                ++mOpenedCount;

            }

        }
@@ -94,20 +98,22 @@ class IdmapDaemon { 
                }



                mOpened = false;

                if (mOpenedCount.decrementAndGet() != 0) {

                if (--mOpenedCount != 0) {

                    // Only post the callback to stop the service if the service does not have an

                    // open connection.

                    return;

                }



                final var service = mService;

                FgThread.getHandler().postDelayed(() -> {

                    synchronized (mIdmapToken) {

                        // Only stop the service if the service does not have an open connection.

                        if (mService == null || mOpenedCount.get() != 0) {

                        // Only stop the service if it's the one we were scheduled for and

                        // it does not have an open connection.

                        if (mService != service || mOpenedCount != 0) {

                            return;

                        }



                        stopIdmapService();

                        stopIdmapServiceLocked();

                        mService = null;

                    }

                }, mIdmapToken, SERVICE_TIMEOUT_MS);
@@ -242,6 +248,7 @@ class IdmapDaemon { 
        } catch (Exception e) {

            Slog.wtf(TAG, "failed to get all fabricated overlays", e);

        } finally {

            if (c != null) {

                try {

                    if (c.getIdmap2() != null && iteratorId != -1) {

                        c.getIdmap2().releaseFabricatedOverlayIterator(iteratorId);
@@ -251,6 +258,7 @@ class IdmapDaemon { 
                }

                c.close();

            }

        }

        return allInfos;

    }
@@ -271,9 +279,11 @@ class IdmapDaemon { 
    }



    @Nullable

    private IBinder getIdmapService() throws TimeoutException, RemoteException {

    private IBinder getIdmapServiceLocked() throws TimeoutException, RemoteException {

        try {

            if (!SystemService.isRunning(IDMAP_DAEMON)) {

                SystemService.start(IDMAP_DAEMON);

            }

        } catch (RuntimeException e) {

            Slog.wtf(TAG, "Failed to enable idmap2 daemon", e);

            if (e.getMessage().contains("failed to set system property")) {
@@ -306,9 +316,11 @@ class IdmapDaemon { 
                        walltimeMillis - endWalltimeMillis + SERVICE_CONNECT_WALLTIME_TIMEOUT_MS));

    }



    private static void stopIdmapService() {

    private static void stopIdmapServiceLocked() {

        try {

            if (SystemService.isRunning(IDMAP_DAEMON)) {

                SystemService.stop(IDMAP_DAEMON);

            }

        } catch (RuntimeException e) {

            // If the idmap daemon cannot be disabled for some reason, it is okay

            // since we already finished invoking idmap.
@@ -326,9 +338,9 @@ class IdmapDaemon { 
                return new Connection(mService);

            }



            IBinder binder = getIdmapService();

            IBinder binder = getIdmapServiceLocked();

            if (binder == null) {

                return new Connection(null);

                return new Connection();

            }



            mService = IIdmap2.Stub.asInterface(binder);