fix(android): retire mpv sessions without blocking hook callbacks

android/libmpv/src/main/cpp/event.cpp

@@ -11,7 +11,8 @@
 // starts (event_thread_bind). Kotlin drops a callback whose session is not
 // the wrapper it published for, so a retiring core's tail — end-file,
 // property changes, a hook raised just before teardown — can never be read
-// as the successor's.
+// as the successor's. L keeps this binding immutable through join, including
+// the interval after admission is revoked; callbacks never acquire L.
 static mpv_handle* thread_mpv;
 static uint64_t thread_session;
 

android/libmpv/src/main/cpp/event.h

@@ -4,10 +4,10 @@
 
 #include <cstdint>
 
-// Names the session the next event_thread serves. Called under the lifecycle
-// write lock before pthread_create, which publishes the values to the thread;
-// the thread then never reads the globals, so a successor session replacing
-// them cannot redirect it.
+// Names the session the next event_thread serves. Called under L and S before
+// pthread_create publishes the binding. L prevents rebinding until that thread
+// is joined; the thread never reads the live admission slots and can safely
+// finish a callback after revocation. Callbacks must never acquire L.
 void event_thread_bind(mpv_handle* mpv, uint64_t session);
 
 void* event_thread(void* arg);

android/libmpv/src/main/cpp/globals.h

@@ -15,10 +15,14 @@ extern JavaVM* g_vm;
 // session it originated from, so a retired wrapper can neither touch nor be
 // fed by its successor.
 //
-// g_session_lock: write-held across create/init/destroy, read-held for every
-// other entry. A reader admitted under a matching session keeps the handle
-// alive until it returns; retirement waits for it, then flips g_session so
-// later callers with the old id are refused before they reach mpv.
+// g_session_lock (S) guards admission: write-held for publication, init and
+// revocation, read-held by every other JNI entry through its last handle use.
+// Retirement drains admitted readers and clears these slots under S, then
+// releases S BEFORE wake/join/terminate so callbacks can reenter and reject.
+// A separate lifecycle mutex (L, private to main.cpp) serializes create/init/
+// destroy through termination AND surface cleanup. Lock order is L -> S;
+// readers and event callbacks never acquire L. The event thread borrows its
+// immutable bound handle until joined, even after the admission slots clear.
 extern mpv_handle* g_mpv;
 extern uint64_t g_session;
 extern pthread_rwlock_t g_session_lock;

android/libmpv/src/main/cpp/main.cpp

@@ -41,20 +41,33 @@ uint64_t g_session;
 pthread_rwlock_t g_session_lock = PTHREAD_RWLOCK_INITIALIZER;
 std::atomic<bool> g_event_thread_request_exit(false);
 
+// Lifecycle serialization (L) is separate from session admission (S).
+// Only nativeCreate/nativeInit/nativeDestroy acquire L, always before S.
+static pthread_mutex_t lifecycle_lock = PTHREAD_MUTEX_INITIALIZER;
 static uint64_t next_session = 0;
 static pthread_t event_thread_id;
 static bool event_thread_started = false;
 
-// Write-locked scope for the lifecycle entries; waits for every admitted
-// reader to return before the handle can change underneath it.
+// Held through retirement, termination and surface cleanup, so a successor
+// cannot overlap or rebind the retiring event thread. Callbacks never take L.
 class LifecycleGuard {
  public:
-  LifecycleGuard() { pthread_rwlock_wrlock(&g_session_lock); }
-  ~LifecycleGuard() { pthread_rwlock_unlock(&g_session_lock); }
+  LifecycleGuard() { pthread_mutex_lock(&lifecycle_lock); }
+  ~LifecycleGuard() { pthread_mutex_unlock(&lifecycle_lock); }
   LifecycleGuard(const LifecycleGuard&) = delete;
   LifecycleGuard& operator=(const LifecycleGuard&) = delete;
 };
 
+// Bounded admission-write scope (S), always nested inside L. Never hold this
+// while joining or terminating: an event callback can reenter SessionGuard.
+class SessionWriteGuard {
+ public:
+  SessionWriteGuard() { pthread_rwlock_wrlock(&g_session_lock); }
+  ~SessionWriteGuard() { pthread_rwlock_unlock(&g_session_lock); }
+  SessionWriteGuard(const SessionWriteGuard&) = delete;
+  SessionWriteGuard& operator=(const SessionWriteGuard&) = delete;
+};
+
 static void prepare_environment(JNIEnv* env, jobject appctx) {
   setlocale(LC_NUMERIC, "C");
 
@@ -66,25 +79,27 @@ static void prepare_environment(JNIEnv* env, jobject appctx) {
   init_methods_cache(env);
 }
 
-// Caller holds the write lock. Stops the event thread, retires the session
-// id (so a late nativeHookContinue / property call for it is refused, not
-// delivered to whatever comes next), and tears the core down. mpv itself
-// continues any hook the destroyed handle was still holding.
-static void destroy_locked(JNIEnv* env) {
+// Caller holds L and S(write). Acquiring S drained every admitted JNI reader;
+// revoke further admission before letting an in-flight callback finish.
+// The lifecycle owner retains the handle and immutable event-thread binding.
+static mpv_handle* revoke_locked() {
   mpv_handle* local_mpv = g_mpv;
+  g_mpv = NULL;
+  g_session = 0;
+  if (event_thread_started) g_event_thread_request_exit = true;
+  return local_mpv;
+}
 
+// Caller holds L, but NOT S. The event thread is the revoked handle's only
+// remaining borrower; a rejected hook can take S and return during this join.
+static void destroy_locked(JNIEnv* env, mpv_handle* local_mpv) {
   // Configuration (including an invalid initial log level) can fail before
   // nativeInit starts the event thread.
   if (event_thread_started) {
-    g_event_thread_request_exit = true;
     mpv_wakeup(local_mpv);
     pthread_join(event_thread_id, NULL);
     event_thread_started = false;
   }
-
-  g_mpv = NULL;
-  g_session = 0;
-
   // The MediaCodec VO can retain the Surface until final decoder teardown.
   // Keep its JNI refs alive for the entire blocking termination.
   mpv_terminate_destroy(local_mpv);
@@ -93,13 +108,20 @@ static void destroy_locked(JNIEnv* env) {
 
 jni_func(jlong, nativeCreate, jobject appctx) {
   LifecycleGuard lock;
-  prepare_environment(env, appctx);
-
-  if (g_mpv) {
+  mpv_handle* predecessor;
+  {
+    SessionWriteGuard admission;
+    predecessor = revoke_locked();
+  }
+  if (predecessor) {
     ALOGE("destroying leaked mpv instance");
-    destroy_locked(env);
+    destroy_locked(env, predecessor);
   }
 
+  // Do not rewrite process-wide JNI state while the predecessor can callback.
+  prepare_environment(env, appctx);
+
+  SessionWriteGuard admission;
   g_mpv = mpv_create();
   if (!g_mpv) {
     die("context init failed");
@@ -113,6 +135,7 @@ jni_func(jlong, nativeCreate, jobject appctx) {
 
 jni_func(jint, nativeInit, jlong session) {
   LifecycleGuard lock;
+  SessionWriteGuard admission;
   if (!g_mpv || g_session != (uint64_t)session) return MPV_ERROR_UNINITIALIZED;
 
   const int result = mpv_initialize(g_mpv);
@@ -140,10 +163,15 @@ jni_func(jint, nativeInit, jlong session) {
 
 jni_func(void, nativeDestroy, jlong session) {
   LifecycleGuard lock;
-  // A wrapper whose session a later nativeCreate already retired has nothing
-  // left to destroy; the successor is not its to touch.
-  if (!g_mpv || g_session != (uint64_t)session) return;
-  destroy_locked(env);
+  mpv_handle* local_mpv;
+  {
+    SessionWriteGuard admission;
+    // A wrapper whose session a later nativeCreate already retired has nothing
+    // left to destroy; the successor is not its to touch.
+    if (!g_mpv || g_session != (uint64_t)session) return;
+    local_mpv = revoke_locked();
+  }
+  destroy_locked(env, local_mpv);
 }
 
 jni_func(jint, nativeSetLogLevel, jlong session, jstring jlevel) {
@@ -202,9 +230,9 @@ jni_func(jlong, nativeCommand, jlong session, jobjectArray jarray) {
   return playlist_entry_id;
 }
 
-// A continuation for a retired session is dropped: the hook id belongs to a
-// handle that no longer exists, and destroying that handle already continued
-// it. The successor must never receive it.
+// A continuation for a revoked session is dropped, even while its handle is
+// still retiring. Destruction releases any outstanding hooks; the successor
+// must never receive an old hook id.
 jni_func(void, nativeHookContinue, jlong session, jlong id) {
   SessionGuard guard(session);
   if (!guard.mpv) return;

android/libmpv/src/main/cpp/render.cpp

@@ -70,8 +70,9 @@ jni_func(void, nativeDetachOsdSurface, jlong session) {
   osd_surface = NULL;
 }
 
-// Caller holds the lifecycle write lock; the surfaces belong to the session
-// being retired and are released with it.
+// Caller holds L after revoking admission, draining JNI readers, joining the
+// event thread and terminating mpv. S is not held. L prevents a successor from
+// publishing new surfaces until these retiring references have been released.
 void render_cleanup(JNIEnv* env) {
   if (surface) {
     env->DeleteGlobalRef(surface);