client API: be more lenient about mpv_suspend/resume mismatches

Before this commit, this was defined to trigger undefined behavior. This
was nice because it required less code; but on the other hand, Lua as
well as IPC support had to check these things manually. Do it directly
in the API to avoid code duplication, and to make the API more robust.
(The total code size still grows, though...)

Since all of the failure cases were originally meant to ruin things
forever, there is no way to return error codes. So just print the
errors.

3 files changed, 53 insertions, 29 deletions

diff --git a/player/client.c b/player/client.c
index a46a37b10a..11a2fda4d8 100644
--- a/player/client.c
+++ b/player/client.c
@@ -108,6 +108,7 @@ struct mpv_handle {
     uint64_t event_mask;
     bool queued_wakeup;
     bool choke_warning;
+    int suspend_count;
 
     mpv_event *events;      // ringbuffer of max_events entries
     int max_events;         // allocated number of entries in events
@@ -300,12 +301,47 @@ void mpv_set_wakeup_callback(mpv_handle *ctx, void (*cb)(void *d), void *d)
 
 void mpv_suspend(mpv_handle *ctx)
 {
-    mp_dispatch_suspend(ctx->mpctx->dispatch);
+    bool do_suspend = false;
+
+    pthread_mutex_lock(&ctx->lock);
+    if (ctx->suspend_count == INT_MAX) {
+        MP_ERR(ctx, "suspend counter overflow");
+    } else {
+        do_suspend = ctx->suspend_count == 0;
+        ctx->suspend_count++;
+    }
+    pthread_mutex_unlock(&ctx->lock);
+
+    if (do_suspend)
+        mp_dispatch_suspend(ctx->mpctx->dispatch);
 }
 
 void mpv_resume(mpv_handle *ctx)
 {
-    mp_dispatch_resume(ctx->mpctx->dispatch);
+    bool do_resume = false;
+
+    pthread_mutex_lock(&ctx->lock);
+    if (ctx->suspend_count == 0) {
+        MP_ERR(ctx, "suspend counter underflow");
+    } else {
+        do_resume = ctx->suspend_count == 1;
+        ctx->suspend_count--;
+    }
+    pthread_mutex_unlock(&ctx->lock);
+
+    if (do_resume)
+        mp_dispatch_resume(ctx->mpctx->dispatch);
+}
+
+void mp_resume_all(mpv_handle *ctx)
+{
+    pthread_mutex_lock(&ctx->lock);
+    bool do_resume = ctx->suspend_count > 0;
+    ctx->suspend_count = 0;
+    pthread_mutex_unlock(&ctx->lock);
+
+    if (do_resume)
+        mp_dispatch_resume(ctx->mpctx->dispatch);
 }
 
 static void lock_core(mpv_handle *ctx)
@@ -325,6 +361,8 @@ void mpv_detach_destroy(mpv_handle *ctx)
     if (!ctx)
         return;
 
+    mp_resume_all(ctx);
+
     pthread_mutex_lock(&ctx->lock);
     // reserved_events equals the number of asynchronous requests that weren't
     // yet replied. In order to avoid that trying to reply to a removed client
@@ -677,6 +715,11 @@ mpv_event *mpv_wait_event(mpv_handle *ctx, double timeout)
     talloc_free_children(event);
 
     while (1) {
+        // This will almost surely lead to a deadlock. (Polling is still ok.)
+        if (ctx->suspend_count && timeout > 0) {
+            MP_ERR(ctx, "attempting to wait while core is suspended");
+            break;
+        }
         if (ctx->num_events) {
             *event = ctx->events[ctx->first_event];
             ctx->first_event = (ctx->first_event + 1) % ctx->max_events;
diff --git a/player/client.h b/player/client.h
index 4e116b3bca..aeb2e886e7 100644
--- a/player/client.h
+++ b/player/client.h
@@ -32,6 +32,8 @@ struct mpv_handle *mp_new_client(struct mp_client_api *clients, const char *name
 struct mp_log *mp_client_get_log(struct mpv_handle *ctx);
 struct MPContext *mp_client_get_core(struct mpv_handle *ctx);
 
+void mp_resume_all(struct mpv_handle *ctx);
+
 // m_option.c
 void *node_get_alloc(struct mpv_node *node);
 
diff --git a/player/lua.c b/player/lua.c
index 1110137c68..47bcc07110 100644
--- a/player/lua.c
+++ b/player/lua.c
@@ -80,7 +80,6 @@ struct script_ctx {
     struct mp_log *log;
     struct mpv_handle *client;
     struct MPContext *mpctx;
-    int suspended;
 };
 
 #if LUA_VERSION_NUM <= 501
@@ -393,8 +392,7 @@ static int load_lua(struct mpv_handle *client, const char *fname)
     r = 0;
 
 error_out:
-    if (ctx->suspended)
-        mpv_resume(ctx->client);
+    mp_resume_all(client);
     if (ctx->state)
         lua_close(ctx->state);
     talloc_free(ctx);
@@ -452,33 +450,20 @@ static int script_find_config_file(lua_State *L)
 static int script_suspend(lua_State *L)
 {
     struct script_ctx *ctx = get_ctx(L);
-    if (!ctx->suspended)
-        mpv_suspend(ctx->client);
-    ctx->suspended++;
+    mpv_suspend(ctx->client);
     return 0;
 }
 
 static int script_resume(lua_State *L)
 {
     struct script_ctx *ctx = get_ctx(L);
-    if (ctx->suspended < 1)
-        luaL_error(L, "trying to resume, but core is not suspended");
-    ctx->suspended--;
-    if (!ctx->suspended)
-        mpv_resume(ctx->client);
+    mpv_resume(ctx->client);
     return 0;
 }
 
-static void resume_all(struct script_ctx *ctx)
-{
-    if (ctx->suspended)
-        mpv_resume(ctx->client);
-    ctx->suspended = 0;
-}
-
 static int script_resume_all(lua_State *L)
 {
-    resume_all(get_ctx(L));
+    mp_resume_all(get_ctx(L)->client);
     return 0;
 }
 
@@ -488,13 +473,7 @@ static int script_wait_event(lua_State *L)
 {
     struct script_ctx *ctx = get_ctx(L);
 
-    double timeout = luaL_optnumber(L, 1, 1e20);
-
-    // This will almost surely lead to a deadlock. (Polling is still ok.)
-    if (ctx->suspended && timeout > 0)
-        luaL_error(L, "attempting to wait while core is suspended");
-
-    mpv_event *event = mpv_wait_event(ctx->client, timeout);
+    mpv_event *event = mpv_wait_event(ctx->client, luaL_optnumber(L, 1, 1e20));
 
     lua_newtable(L); // event
     lua_pushstring(L, mpv_event_name(event->event_id)); // event name
@@ -1196,7 +1175,7 @@ static int script_subprocess(lua_State *L)
     luaL_checktype(L, 1, LUA_TTABLE);
     void *tmp = mp_lua_PITA(L);
 
-    resume_all(ctx);
+    mp_resume_all(ctx->client);
 
     lua_getfield(L, 1, "args"); // args
     int num_args = mp_lua_len(L, -1);