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.

4 files changed, 57 insertions, 45 deletions

diff --git a/input/ipc.c b/input/ipc.c
index 876f40c8a7..7d15ac84d0 100644
--- a/input/ipc.c
+++ b/input/ipc.c
@@ -61,8 +61,6 @@ struct client_arg {
     bool close_client_fd;
 
     bool writable;
-
-    int suspend_counter;
 };
 
 static mpv_node *mpv_node_map_get(mpv_node *src, const char *key)
@@ -420,21 +418,11 @@ static char *json_execute_command(struct client_arg *arg, void *ta_parent,
         rc = mpv_request_log_messages(arg->client,
                                       cmd_node->u.list->values[1].u.string);
     } else if (!strcmp("suspend", cmd)) {
-        if (arg->suspend_counter < INT_MAX) {
-            mpv_suspend(arg->client);
-            arg->suspend_counter++;
-            rc = MPV_ERROR_SUCCESS;
-        } else {
-            rc = MPV_ERROR_INVALID_PARAMETER;
-        }
+        mpv_suspend(arg->client);
+        rc = MPV_ERROR_SUCCESS;
     } else if (!strcmp("resume", cmd)) {
-        if (arg->suspend_counter > 0) {
-            mpv_resume(arg->client);
-            arg->suspend_counter--;
-            rc = MPV_ERROR_SUCCESS;
-        } else {
-            rc = MPV_ERROR_INVALID_PARAMETER;
-        }
+        mpv_resume(arg->client);
+        rc = MPV_ERROR_SUCCESS;
     } else {
         mpv_node result_node;
 
diff --git a/player/client.c b/player/client.c
index 7e9d9b5247..8cd47df4ab 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
@@ -674,6 +712,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 a275bb9728..43d75ae413 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 7f7fb69bce..95dd11acf7 100644
--- a/player/lua.c
+++ b/player/lua.c
@@ -81,7 +81,6 @@ struct script_ctx {
     struct mp_log *log;
     struct mpv_handle *client;
     struct MPContext *mpctx;
-    int suspended;
 };
 
 #if LUA_VERSION_NUM <= 501
@@ -388,8 +387,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);
@@ -447,33 +445,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;
 }
 
@@ -483,13 +468,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
@@ -1191,7 +1170,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);