[Android] retain trace replay ownership across Activity recreation

android-plugin/app/src/trace/AndroidManifest.xml

@@ -52,8 +52,11 @@
             android:name="pojavEnv"
             tools:node="remove" />
 
+        <!-- Keep the native render Surface through runtime overlay/display changes.
+             See the integer resource for the API 34-compatible configuration mask. -->
         <activity
             android:name=".trace.TraceReplayActivity"
+            android:configChanges="@integer/trace_replay_config_changes"
             android:exported="true"
             android:theme="@style/Theme.MobileGLPlugin">
             <intent-filter>

android-plugin/app/src/trace/cpp/trace_replay_jni.cpp

@@ -2,6 +2,7 @@
 
 #include "spawn_spike.hpp"
 #include "trace_env_overrides.hpp"
+#include "trace_replay_lease.hpp"
 
 #include <android/native_window.h>
 #include <android/native_window_jni.h>
@@ -119,6 +120,18 @@ Java_top_mobilegl_plugin_trace_TraceReplayActivity_nativeRunTraceReplay(JNIEnv*
                                                                         jboolean benchmarkFinish,
                                                                         jstring benchmarkResultPath,
                                                                         jstring envOverrides) {
+    // Claim before changing GLWS globals, environment, logs or output files. The lease
+    // outlives replayState, so another invocation cannot race native-window cleanup.
+    mobilegl_trace::TraceReplayLease lease;
+    if (!lease) {
+        mobilegl_trace::Result rejected;
+        rejected.statusCode = mobilegl_trace::STATUS_RETRACE_FAILED;
+        rejected.message = "another trace replay is already running in this process";
+        // No paths: a duplicate Activity can share outputDir with the live invocation.
+        // Publishing this failure there would overwrite that invocation's real result.
+        return MakeResult(env, rejected);
+    }
+
     mobilegl_trace::Request request;
     request.tracePath = ToString(env, tracePath);
     request.goldenPath = ToString(env, goldenPath);
@@ -157,12 +170,20 @@ Java_top_mobilegl_plugin_trace_TraceReplayActivity_nativeRunTraceReplay(JNIEnv*
     request.benchmarkResultPath = ToString(env, benchmarkResultPath);
     request.envOverrides = SplitSemicolonList(ToString(env, envOverrides));
 
-    ScopedTraceReplayState replayState;
-    mobilegl_trace_set_requested_size(request.width, request.height);
     const bool needsNativeWindow =
         request.backend == "DirectVulkan" ||
         (request.backend == "DirectGLES" && !request.usePbuffer);
     ANativeWindow *window = needsNativeWindow && surface != nullptr ? ANativeWindow_fromSurface(env, surface) : nullptr;
+    if (needsNativeWindow && window == nullptr) {
+        auto result = MakeFailureResult(request, mobilegl_trace::STATUS_RETRACE_FAILED,
+                                       "render surface was destroyed before native replay started");
+        if (!result.resultPath.empty()) {
+            mobilegl_trace::WriteResultJson(request, result);
+        }
+        return MakeResult(env, result);
+    }
+    ScopedTraceReplayState replayState;
+    mobilegl_trace_set_requested_size(request.width, request.height);
     mobilegl_trace_set_native_window(window);
     if (window != nullptr) {
         ANativeWindow_release(window);

android-plugin/app/src/trace/cpp/trace_replay_lease.hpp

@@ -0,0 +1,28 @@
+#pragma once
+
+#include <atomic>
+
+namespace mobilegl_trace {
+
+// apitrace, its GLWS adapter and the environment are process-global. --singlethread
+// controls trace dispatch only; it does not make two JNI invocations independent.
+class TraceReplayLease {
+public:
+    TraceReplayLease() : acquired_(!running_.test_and_set(std::memory_order_acquire)) {}
+    ~TraceReplayLease() {
+        if (acquired_) {
+            running_.clear(std::memory_order_release);
+        }
+    }
+
+    TraceReplayLease(const TraceReplayLease&) = delete;
+    TraceReplayLease& operator=(const TraceReplayLease&) = delete;
+
+    explicit operator bool() const { return acquired_; }
+
+private:
+    inline static std::atomic_flag running_ = ATOMIC_FLAG_INIT;
+    const bool acquired_;
+};
+
+} // namespace mobilegl_trace

android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplayActivity.java

@@ -25,7 +25,8 @@ public final class TraceReplayActivity extends Activity {
 
     private TextView statusView;
     private TraceReplayRequest request;
-    private boolean started;
+    private TraceReplaySession<TraceReplayResult> replaySession;
+    private final TraceReplaySession.Listener<TraceReplayResult> replayListener = this::onReplayComplete;
     private final Handler mainHandler = new Handler(Looper.getMainLooper());
 
     @Override
@@ -62,6 +63,18 @@ public final class TraceReplayActivity extends Activity {
             return;
         }
 
+        @SuppressWarnings("unchecked")
+        TraceReplaySession<TraceReplayResult> retained =
+                (TraceReplaySession<TraceReplayResult>) getLastNonConfigurationInstance();
+        replaySession = retained != null ? retained : new TraceReplaySession<>(
+                command -> new Thread(command, "MobileGLTraceReplay").start(),
+                command -> new Handler(Looper.getMainLooper()).post(command));
+        if (replaySession.hasStarted()) {
+            Log.i(TAG, "Reattaching to retained trace replay: " + request.outputDir);
+            statusView.setText("Running trace replay\n" + request.outputDir);
+        }
+        replaySession.attach(replayListener);
+
         SurfaceHolder holder = surfaceView.getHolder();
         if (request.width > 0 && request.height > 0) {
             holder.setFixedSize(request.width, request.height);
@@ -87,20 +100,43 @@ public final class TraceReplayActivity extends Activity {
         mainHandler.postDelayed(() -> startReplay(holder), 250);
     }
 
+    @Override
+    public Object onRetainNonConfigurationInstance() {
+        return replaySession;
+    }
+
+    @Override
+    protected void onDestroy() {
+        // A callback queued by an old Surface must not start after that Activity is gone.
+        mainHandler.removeCallbacksAndMessages(null);
+        if (replaySession != null) {
+            replaySession.detach(replayListener);
+        }
+        super.onDestroy();
+    }
+
     private void startReplay(SurfaceHolder holder) {
-        if (started) {
+        if (isFinishing() || isDestroyed() || replaySession.hasStarted()) {
             return;
         }
         Surface surface = holder.getSurface();
         if (surface == null || !surface.isValid()) {
             return;
         }
-        started = true;
         statusView.setText("Running trace replay\n" + request.outputDir);
-        new Thread(() -> runRequest(request, surface), "MobileGLTraceReplay").start();
+        TraceReplayRequest replayRequest = request;
+        // The worker retains the request and Surface, not the Activity. JNI also acquires
+        // its own ANativeWindow reference until the replay and native cleanup finish.
+        replaySession.start(() -> runRequest(replayRequest, surface));
     }
 
-    private void runRequest(TraceReplayRequest request, Surface surface) {
+    private void onReplayComplete(TraceReplayResult result) {
+        statusView.setText(result.toString());
+        finish();
+    }
+
+    private static TraceReplayResult runRequest(TraceReplayRequest request, Surface surface) {
+        Log.i(TAG, "Starting native trace replay: " + request.outputDir);
         TraceReplayResult result = nativeRunTraceReplay(
                 surface,
                 request.tracePath,
@@ -135,11 +171,7 @@ public final class TraceReplayActivity extends Activity {
                 request.envOverrides
         );
         Log.i(TAG, result.toString());
-        TraceReplayResult finalResult = result;
-        runOnUiThread(() -> {
-            statusView.setText(finalResult.toString());
-            finish();
-        });
+        return result;
     }
 
     private static native TraceReplayResult nativeRunTraceReplay(
@@ -202,9 +234,7 @@ public final class TraceReplayActivity extends Activity {
     }
 
     private void runSpawnSpike(String libraryName) {
-        // The surface callbacks fire regardless; this keeps them from starting a replay
-        // underneath the spike.
-        started = true;
+        // onCreate returns before installing replay surface callbacks in this mode.
         File outputDir = new File(request.outputDir);
         String serverPath = new File(getApplicationInfo().nativeLibraryDir, libraryName)
                 .getAbsolutePath();

android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplaySession.java

@@ -0,0 +1,59 @@
+package top.mobilegl.plugin.trace;
+
+import java.util.concurrent.Executor;
+import java.util.function.Supplier;
+
+/** One native invocation, retained when Android replaces the Activity that displays it. */
+final class TraceReplaySession<T> {
+    interface Listener<T> {
+        void onComplete(T result);
+    }
+
+    private final Executor worker;
+    private final Executor completion;
+    private boolean started;
+    private boolean completed;
+    private T result;
+    private Listener<T> listener;
+
+    TraceReplaySession(Executor worker, Executor completion) {
+        this.worker = worker;
+        this.completion = completion;
+    }
+
+    // All session state is accessed on the main thread. Only replay.get() runs on the worker.
+    boolean hasStarted() {
+        return started;
+    }
+
+    boolean start(Supplier<T> replay) {
+        if (started) {
+            return false;
+        }
+        started = true;
+        worker.execute(() -> {
+            T replayResult = replay.get();
+            completion.execute(() -> {
+                result = replayResult;
+                completed = true;
+                if (listener != null) {
+                    listener.onComplete(result);
+                }
+            });
+        });
+        return true;
+    }
+
+    void attach(Listener<T> newListener) {
+        listener = newListener;
+        if (completed) {
+            listener.onComplete(result);
+        }
+    }
+
+    void detach(Listener<T> oldListener) {
+        if (listener == oldListener) {
+            listener = null;
+        }
+    }
+}

android-plugin/app/src/trace/res/values/config.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+    <!-- CONFIG_ASSETS_PATHS (0x80000000) exists in Android 34, but that SDK has no
+         public configChanges flag name for it. Package parsing resolves this integer
+         through TypedArray.getInt. Keep it unqualified: overlay installation must not
+         recreate the Activity and destroy the live native render Surface.
+         0x0f80 = orientation | screenLayout | uiMode | screenSize | smallestScreenSize,
+         matching PluginActivity. Trace framebuffer dimensions are request parameters,
+         and these configuration changes need no new view or native replay. -->
+    <integer name="trace_replay_config_changes">0x80000f80</integer>
+</resources>

tools/trace_replay/CMakeLists.txt

@@ -298,6 +298,17 @@ if(NOT CMAKE_CROSSCOMPILING)
             COMMAND mobilegl_trace_env_overrides_test
             COMMENT "Checking the retrace env passthrough parser")
     add_dependencies(mobilegl_trace_replay mobilegl_trace_env_overrides_test)
+
+    add_executable(mobilegl_trace_replay_lease_test
+            "${MOBILEGL_TRACE_ROOT}/trace_replay_lease_test.cpp")
+    target_compile_features(mobilegl_trace_replay_lease_test PRIVATE cxx_std_17)
+    target_include_directories(mobilegl_trace_replay_lease_test PRIVATE
+            "${MOBILEGL_TRACE_SHARED_CPP_DIR}")
+    target_link_libraries(mobilegl_trace_replay_lease_test PRIVATE Threads::Threads)
+    add_custom_command(TARGET mobilegl_trace_replay_lease_test POST_BUILD
+            COMMAND mobilegl_trace_replay_lease_test
+            COMMENT "Checking process ownership of native trace replay")
+    add_dependencies(mobilegl_trace_replay mobilegl_trace_replay_lease_test)
 endif()
 
 if(MOBILEGL_TRACE_REPLAY_MOBILEGL_LIBRARY)
@@ -464,4 +475,4 @@ execute_process(
         --fixture-root "${MOBILEGL_TRACE_ROOT}/fixtures"
         OUTPUT_FILE "${MOBILEGL_TRACE_CASES_CMAKE}"
         COMMAND_ERROR_IS_FATAL ANY)
-include("${MOBILEGL_TRACE_CASES_CMAKE}")
+include("${MOBILEGL_TRACE_CASES_CMAKE}")

tools/trace_replay/TraceReplaySessionTest.java

@@ -0,0 +1,119 @@
+package top.mobilegl.plugin.trace;
+
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
+
+/** Deterministic Activity replacement while native work or its completion is pending. */
+public final class TraceReplaySessionTest {
+    private static final class Queue implements Executor {
+        final ArrayDeque<Runnable> commands = new ArrayDeque<>();
+
+        @Override
+        public void execute(Runnable command) {
+            commands.add(command);
+        }
+
+        void drain() {
+            while (!commands.isEmpty()) {
+                commands.remove().run();
+            }
+        }
+    }
+
+    private static void require(boolean condition, String message) {
+        if (!condition) {
+            throw new AssertionError(message);
+        }
+    }
+
+    public static void main(String[] args) throws InterruptedException {
+        Queue worker = new Queue();
+        Queue main = new Queue();
+        TraceReplaySession<String> session = new TraceReplaySession<>(worker, main);
+        List<String> oldResults = new ArrayList<>();
+        List<String> newResults = new ArrayList<>();
+        TraceReplaySession.Listener<String> oldActivity = oldResults::add;
+        TraceReplaySession.Listener<String> newActivity = newResults::add;
+        int[] nativeCalls = {0};
+
+        // Recreation before a valid Surface: the replacement may start the first run.
+        session.attach(oldActivity);
+        session.detach(oldActivity);
+        session.attach(newActivity);
+        require(session.start(() -> { ++nativeCalls[0]; return "real native result"; }),
+                "replacement must be able to start a pending request");
+        require(session.hasStarted(), "ownership must be published before worker dispatch");
+
+        // surfaceCreated and surfaceChanged can both queue callbacks, and a recreated
+        // Activity receives both again. None may dispatch another native invocation.
+        session.detach(newActivity);
+        session.attach(oldActivity);
+        require(!session.start(() -> { ++nativeCalls[0]; return "duplicate"; }),
+                "Activity recreation must retain the running invocation");
+        worker.drain();
+        require(nativeCalls[0] == 1, "only one worker may execute the native request");
+
+        // The native result can arrive between the old Activity's destruction and the
+        // replacement attaching. It must be kept, without calling a destroyed owner.
+        session.detach(oldActivity);
+        main.drain();
+        require(oldResults.isEmpty() && newResults.isEmpty(), "destroyed owners must not be notified");
+        session.attach(newActivity);
+        require(newResults.equals(List.of("real native result")), "replacement must receive original result");
+        require(!session.start(() -> "replayed completed request"), "completed request must not replay");
+
+        // An obsolete owner must not detach the current owner, including while running.
+        TraceReplaySession<String> next = new TraceReplaySession<>(worker, main);
+        next.attach(oldActivity);
+        next.attach(newActivity);
+        next.detach(oldActivity);
+        next.start(() -> "next native result");
+        worker.drain();
+        main.drain();
+        require(oldResults.isEmpty(), "obsolete owner was notified");
+        require(newResults.equals(List.of("real native result", "next native result")),
+                "late detach lost the current owner");
+
+        // Replace the observer while the worker is actually inside the native-call
+        // boundary, not just queued. Latches force the interleaving without sleeps.
+        CountDownLatch entered = new CountDownLatch(1);
+        CountDownLatch release = new CountDownLatch(1);
+        List<Thread> threads = new ArrayList<>();
+        TraceReplaySession<String> running = new TraceReplaySession<>(command -> {
+            Thread thread = new Thread(command);
+            threads.add(thread);
+            thread.start();
+        }, main);
+        running.attach(oldActivity);
+        running.start(() -> {
+            entered.countDown();
+            try {
+                if (!release.await(5, TimeUnit.SECONDS)) {
+                    throw new AssertionError("test did not release the native invocation");
+                }
+            } catch (InterruptedException exception) {
+                throw new AssertionError(exception);
+            }
+            return "running native result";
+        });
+        require(entered.await(5, TimeUnit.SECONDS), "native worker did not enter");
+        running.detach(oldActivity);
+        running.attach(newActivity);
+        boolean duplicate = running.start(() -> "duplicate while running");
+        release.countDown();
+        for (Thread thread : threads) {
+            thread.join(5000);
+            require(!thread.isAlive(), "native worker did not finish");
+        }
+        require(!duplicate, "recreation entered native code while original invocation was active");
+        main.drain();
+        require(oldResults.isEmpty(), "running invocation notified destroyed Activity");
+        require(newResults.equals(List.of("real native result", "next native result", "running native result")),
+                "running invocation lost its original result during Activity replacement");
+        System.out.println("TraceReplaySession: pending, running and completed recreation passed");
+    }
+}

tools/trace_replay/test_android_lifecycle.py

@@ -0,0 +1,39 @@
+#!/usr/bin/env python3
+"""Run the Android replay ownership regression without an emulator or Gradle."""
+
+import pathlib
+import subprocess
+import tempfile
+import xml.etree.ElementTree as ET
+
+
+def main():
+    root = pathlib.Path(__file__).resolve().parents[2]
+    trace = root / "android-plugin/app/src/trace"
+    android = "{http://schemas.android.com/apk/res/android}"
+    manifest = ET.parse(trace / "AndroidManifest.xml")
+    activity = next(a for a in manifest.findall("application/activity")
+                    if a.get(android + "name") == ".trace.TraceReplayActivity")
+    config_value = activity.get(android + "configChanges", "0")
+    if config_value == "@integer/trace_replay_config_changes":
+        resources = ET.parse(trace / "res/values/config.xml")
+        config_value = resources.find("integer[@name='trace_replay_config_changes']").text
+    changes = int(config_value, 0)
+    # The failing emulator added resource overlays after replay had already started.
+    # orientation/screenSize alone does not handle CONFIG_ASSETS_PATHS.
+    if changes & 0x80000000 == 0:
+        raise RuntimeError("TraceReplayActivity must handle CONFIG_ASSETS_PATHS")
+    if changes & 0xF80 != 0xF80:
+        raise RuntimeError("TraceReplayActivity must retain the fixed-size render surface on display changes")
+    with tempfile.TemporaryDirectory(prefix="mobilegl-replay-lifecycle-") as output:
+        subprocess.run([
+            "javac", "--release", "11", "-d", output,
+            str(trace / "java/top/mobilegl/plugin/trace/TraceReplaySession.java"),
+            str(root / "tools/trace_replay/TraceReplaySessionTest.java"),
+        ], check=True)
+        subprocess.run(["java", "-cp", output, "top.mobilegl.plugin.trace.TraceReplaySessionTest"], check=True)
+    print("Android replay manifest and lifecycle checks passed")
+
+
+if __name__ == "__main__":
+    main()

tools/trace_replay/trace_replay_lease_test.cpp

@@ -0,0 +1,46 @@
+#include "trace_replay_lease.hpp"
+
+#include <iostream>
+#include <thread>
+#include <vector>
+
+int main() {
+    using mobilegl_trace::TraceReplayLease;
+    std::atomic<int> rejected{0};
+    {
+        TraceReplayLease original;
+        if (!original) {
+            std::cerr << "initial replay could not claim process state\n";
+            return 1;
+        }
+        std::vector<std::thread> contenders;
+        for (int i = 0; i < 16; ++i) {
+            contenders.emplace_back([&] {
+                TraceReplayLease duplicate;
+                if (!duplicate) {
+                    ++rejected;
+                }
+            });
+        }
+        for (auto& thread : contenders) {
+            thread.join();
+        }
+        if (rejected != 16) {
+            std::cerr << "a duplicate replay entered the live process state\n";
+            return 1;
+        }
+        // A rejected caller's destructor must not release the real owner's claim.
+        TraceReplayLease stillDuplicate;
+        if (stillDuplicate) {
+            std::cerr << "rejected caller released the original replay's claim\n";
+            return 1;
+        }
+    }
+    TraceReplayLease next;
+    if (!next) {
+        std::cerr << "completed replay did not release process state\n";
+        return 1;
+    }
+    std::cout << "TraceReplayLease: concurrent invocations rejected; owner cleanup retained\n";
+    return 0;
+}