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

Commit 95b9d937 authored by Hawkwood Glazier's avatar Hawkwood Glazier Committed by Android (Google) Code Review
Browse files

Merge "Synchronize PluginInstance mutation methods" into main

parents e940d8d0 cb8787e8
Loading
Loading
Loading
Loading
+5 −4
Original line number Diff line number Diff line
@@ -101,7 +101,7 @@ public class PluginInstance<T extends Plugin> implements PluginLifecycleManager
    }

    /** Alerts listener and plugin that the plugin has been created. */
    public void onCreate() {
    public synchronized void onCreate() {
        boolean loadPlugin = mListener.onPluginAttached(this);
        if (!loadPlugin) {
            if (mPlugin != null) {
@@ -128,7 +128,7 @@ public class PluginInstance<T extends Plugin> implements PluginLifecycleManager
    }

    /** Alerts listener and plugin that the plugin is being shutdown. */
    public void onDestroy() {
    public synchronized void onDestroy() {
        logDebug("onDestroy");
        unloadPlugin();
        mListener.onPluginDetached(this);
@@ -143,12 +143,13 @@ public class PluginInstance<T extends Plugin> implements PluginLifecycleManager
    /**
     * Loads and creates the plugin if it does not exist.
     */
    public void loadPlugin() {
    public synchronized void loadPlugin() {
        if (mPlugin != null) {
            logDebug("Load request when already loaded");
            return;
        }

        // Both of these calls take about 1 - 1.5 seconds in test runs
        mPlugin = mPluginFactory.createPlugin();
        mPluginContext = mPluginFactory.createPluginContext();
        if (mPlugin == null || mPluginContext == null) {
@@ -171,7 +172,7 @@ public class PluginInstance<T extends Plugin> implements PluginLifecycleManager
     *
     * This will free the associated memory if there are not other references.
     */
    public void unloadPlugin() {
    public synchronized void unloadPlugin() {
        if (mPlugin == null) {
            logDebug("Unload request when already unloaded");
            return;
+83 −3
Original line number Diff line number Diff line
@@ -17,13 +17,17 @@
package com.android.systemui.shared.plugins;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertNotNull;
import static junit.framework.Assert.assertNull;
import static junit.framework.Assert.assertTrue;
import static junit.framework.Assert.fail;

import android.content.ComponentName;
import android.content.Context;
import android.content.pm.ApplicationInfo;
import android.test.suitebuilder.annotation.SmallTest;
import android.util.Log;

import androidx.test.runner.AndroidJUnit4;

@@ -40,7 +44,11 @@ import org.junit.runner.RunWith;

import java.lang.ref.WeakReference;
import java.util.Collections;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;

@SmallTest
@RunWith(AndroidJUnit4.class)
@@ -104,6 +112,7 @@ public class PluginInstanceTest extends SysuiTestCase {
        mPluginInstance = mPluginInstanceFactory.create(
                mContext, mAppInfo, TEST_PLUGIN_COMPONENT_NAME,
                TestPlugin.class, mPluginListener);
        mPluginInstance.setIsDebug(true);
        mPluginContext = new WeakReference<>(mPluginInstance.getPluginContext());
    }

@@ -158,7 +167,7 @@ public class PluginInstanceTest extends SysuiTestCase {

    @Test
    public void testOnAttach_SkipLoad() {
        mPluginListener.mAttachReturn = false;
        mPluginListener.mOnAttach = () -> false;
        mPluginInstance.onCreate();
        assertEquals(1, mPluginListener.mAttachedCount);
        assertEquals(0, mPluginListener.mLoadCount);
@@ -166,6 +175,65 @@ public class PluginInstanceTest extends SysuiTestCase {
        assertInstances(0, 0);
    }

    @Test
    public void testLoadUnloadSimultaneous_HoldsUnload() throws Exception {
        final Semaphore loadLock = new Semaphore(1);
        final Semaphore unloadLock = new Semaphore(1);

        mPluginListener.mOnAttach = () -> false;
        mPluginListener.mOnLoad = () -> {
            assertNotNull(mPluginInstance.getPlugin());

            // Allow the bg thread the opportunity to delete the plugin
            loadLock.release();
            Thread.yield();
            boolean isLocked = getLock(unloadLock, 1000);

            // Ensure the bg thread failed to do delete the plugin
            assertNotNull(mPluginInstance.getPlugin());
            // We expect that bgThread deadlocked holding the semaphore
            assertFalse(isLocked);
        };

        AtomicBoolean isBgThreadFailed = new AtomicBoolean(false);
        Thread bgThread = new Thread(() -> {
            assertTrue(getLock(unloadLock, 10));
            assertTrue(getLock(loadLock, 3000)); // Wait for the foreground thread
            assertNotNull(mPluginInstance.getPlugin());
            // Attempt to delete the plugin, this should block until the load completes
            mPluginInstance.unloadPlugin();
            assertNull(mPluginInstance.getPlugin());
            unloadLock.release();
            loadLock.release();
        });

        // This protects the test suite from crashing due to the uncaught exception.
        bgThread.setUncaughtExceptionHandler((Thread t, Throwable ex) -> {
            Log.e("testLoadUnloadSimultaneous_HoldsUnload", "Exception from BG Thread", ex);
            isBgThreadFailed.set(true);
        });

        loadLock.acquire();
        mPluginInstance.onCreate();

        assertNull(mPluginInstance.getPlugin());
        bgThread.start();
        mPluginInstance.loadPlugin();

        bgThread.join(5000);
        assertFalse(isBgThreadFailed.get());
        assertNull(mPluginInstance.getPlugin());
    }

    private boolean getLock(Semaphore lock, long millis) {
        try {
            return lock.tryAcquire(millis, TimeUnit.MILLISECONDS);
        } catch (InterruptedException ex) {
            fail();
            return false;
        }
    }

    // This target class doesn't matter, it just needs to have a Requires to hit the flow where
    // the mock version info is called.
    @ProvidesInterface(action = TestPlugin.ACTION, version = TestPlugin.VERSION)
@@ -226,7 +294,10 @@ public class PluginInstanceTest extends SysuiTestCase {
    }

    public class FakeListener implements PluginListener<TestPlugin> {
        public boolean mAttachReturn = true;
        public Supplier<Boolean> mOnAttach = null;
        public Runnable mOnDetach = null;
        public Runnable mOnLoad = null;
        public Runnable mOnUnload = null;
        public int mAttachedCount = 0;
        public int mDetachedCount = 0;
        public int mLoadCount = 0;
@@ -236,13 +307,16 @@ public class PluginInstanceTest extends SysuiTestCase {
        public boolean onPluginAttached(PluginLifecycleManager<TestPlugin> manager) {
            mAttachedCount++;
            assertEquals(PluginInstanceTest.this.mPluginInstance, manager);
            return mAttachReturn;
            return mOnAttach != null ? mOnAttach.get() : true;
        }

        @Override
        public void onPluginDetached(PluginLifecycleManager<TestPlugin> manager) {
            mDetachedCount++;
            assertEquals(PluginInstanceTest.this.mPluginInstance, manager);
            if (mOnDetach != null) {
                mOnDetach.run();
            }
        }

        @Override
@@ -261,6 +335,9 @@ public class PluginInstanceTest extends SysuiTestCase {
                assertEquals(expectedContext, pluginContext);
            }
            assertEquals(PluginInstanceTest.this.mPluginInstance, manager);
            if (mOnLoad != null) {
                mOnLoad.run();
            }
        }

        @Override
@@ -274,6 +351,9 @@ public class PluginInstanceTest extends SysuiTestCase {
                assertEquals(expectedPlugin, plugin);
            }
            assertEquals(PluginInstanceTest.this.mPluginInstance, manager);
            if (mOnUnload != null) {
                mOnUnload.run();
            }
        }
    }
}