From 9bda301eb4d8aa75d435f24146c3a66ab74cafea Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Thu, 26 Mar 2020 23:40:25 +0100
Subject: command: use client IDs for hooks

Removes weird potential race conditions when a client is removed and a
new one with the same name is created.
---
 player/client.c  | 12 ++++++++++--
 player/client.h  |  1 +
 player/command.c | 17 ++++++++++-------
 player/command.h |  6 +++---
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/player/client.c b/player/client.c
index 41645b5d95..a72cb0125f 100644
--- a/player/client.c
+++ b/player/client.c
@@ -262,6 +262,14 @@ bool mp_client_exists(struct MPContext *mpctx, const char *client_name)
     return r;
 }
 
+bool mp_client_id_exists(struct MPContext *mpctx, int64_t id)
+{
+    pthread_mutex_lock(&mpctx->clients->lock);
+    bool r = find_client_id(mpctx->clients, id);
+    pthread_mutex_unlock(&mpctx->clients->lock);
+    return r;
+}
+
 struct mpv_handle *mp_new_client(struct mp_client_api *clients, const char *name)
 {
     pthread_mutex_lock(&clients->lock);
@@ -1825,7 +1833,7 @@ int mpv_hook_add(mpv_handle *ctx, uint64_t reply_userdata,
                  const char *name, int priority)
 {
     lock_core(ctx);
-    mp_hook_add(ctx->mpctx, ctx->name, name, reply_userdata, priority);
+    mp_hook_add(ctx->mpctx, ctx->name, ctx->id, name, reply_userdata, priority);
     unlock_core(ctx);
     return 0;
 }
@@ -1833,7 +1841,7 @@ int mpv_hook_add(mpv_handle *ctx, uint64_t reply_userdata,
 int mpv_hook_continue(mpv_handle *ctx, uint64_t id)
 {
     lock_core(ctx);
-    int r = mp_hook_continue(ctx->mpctx, ctx->name, id);
+    int r = mp_hook_continue(ctx->mpctx, ctx->id, id);
     unlock_core(ctx);
     return r;
 }
diff --git a/player/client.h b/player/client.h
index a8eae2d0c1..47af084cc5 100644
--- a/player/client.h
+++ b/player/client.h
@@ -24,6 +24,7 @@ bool mp_is_shutting_down(struct MPContext *mpctx);
 bool mp_clients_all_initialized(struct MPContext *mpctx);
 
 bool mp_client_exists(struct MPContext *mpctx, const char *client_name);
+bool mp_client_id_exists(struct MPContext *mpctx, int64_t id);
 void mp_client_broadcast_event(struct MPContext *mpctx, int event, void *data);
 int mp_client_send_event(struct MPContext *mpctx, const char *client_name,
                          uint64_t reply_userdata, int event, void *data);
diff --git a/player/command.c b/player/command.c
index 5984d6c5b3..3098e1d9bf 100644
--- a/player/command.c
+++ b/player/command.c
@@ -118,7 +118,8 @@ struct overlay {
 };
 
 struct hook_handler {
-    char *client;   // client API user name
+    char *client;   // client mpv_handle name (for logging)
+    int64_t client_id; // client mpv_handle ID
     char *type;     // kind of hook, e.g. "on_load"
     uint64_t user_id; // user-chosen ID
     int priority;   // priority for global hook order
@@ -160,7 +161,7 @@ bool mp_hook_test_completion(struct MPContext *mpctx, char *type)
     for (int n = 0; n < cmd->num_hooks; n++) {
         struct hook_handler *h = cmd->hooks[n];
         if (h->active && strcmp(h->type, type) == 0) {
-            if (!mp_client_exists(mpctx, h->client)) {
+            if (!mp_client_id_exists(mpctx, h->client_id)) {
                 MP_WARN(mpctx, "client removed during hook handling\n");
                 hook_remove(mpctx, h);
                 break;
@@ -183,7 +184,8 @@ static int invoke_hook_handler(struct MPContext *mpctx, struct hook_handler *h)
         .id = h->seq,
     },
     reply_id = h->user_id;
-    int r = mp_client_send_event(mpctx, h->client, reply_id, MPV_EVENT_HOOK, m);
+    char *name = mp_tprintf(22, "@%"PRIi64, h->client_id);
+    int r = mp_client_send_event(mpctx, name, reply_id, MPV_EVENT_HOOK, m);
     if (r < 0) {
         MP_WARN(mpctx, "Sending hook command failed. Removing hook.\n");
         hook_remove(mpctx, h);
@@ -216,13 +218,13 @@ void mp_hook_start(struct MPContext *mpctx, char *type)
     }
 }
 
-int mp_hook_continue(struct MPContext *mpctx, char *client, uint64_t id)
+int mp_hook_continue(struct MPContext *mpctx, int64_t client_id, uint64_t id)
 {
     struct command_ctx *cmd = mpctx->command_ctx;
 
     for (int n = 0; n < cmd->num_hooks; n++) {
         struct hook_handler *h = cmd->hooks[n];
-        if (strcmp(h->client, client) == 0 && h->seq == id) {
+        if (h->client_id == client_id && h->seq == id) {
             if (!h->active)
                 break;
             h->active = false;
@@ -243,14 +245,15 @@ static int compare_hook(const void *pa, const void *pb)
     return (*h1)->seq - (*h2)->seq;
 }
 
-void mp_hook_add(struct MPContext *mpctx, const char *client, const char *name,
-                 uint64_t user_id, int pri)
+void mp_hook_add(struct MPContext *mpctx, char *client, int64_t client_id,
+                 const char *name, uint64_t user_id, int pri)
 {
     struct command_ctx *cmd = mpctx->command_ctx;
     struct hook_handler *h = talloc_ptrtype(cmd, h);
     int64_t seq = ++cmd->hook_seq;
     *h = (struct hook_handler){
         .client = talloc_strdup(h, client),
+        .client_id = client_id,
         .type = talloc_strdup(h, name),
         .user_id = user_id,
         .priority = pri,
diff --git a/player/command.h b/player/command.h
index 1b3154394a..17e0726b0b 100644
--- a/player/command.h
+++ b/player/command.h
@@ -105,9 +105,9 @@ enum {
 
 bool mp_hook_test_completion(struct MPContext *mpctx, char *type);
 void mp_hook_start(struct MPContext *mpctx, char *type);
-int mp_hook_continue(struct MPContext *mpctx, char *client, uint64_t id);
-void mp_hook_add(struct MPContext *mpctx, const char *client, const char *name,
-                 uint64_t user_id, int pri);
+int mp_hook_continue(struct MPContext *mpctx, int64_t client_id, uint64_t id);
+void mp_hook_add(struct MPContext *mpctx, char *client, int64_t client_id,
+                 const char *name, uint64_t user_id, int pri);
 
 void mark_seek(struct MPContext *mpctx);
 
-- 
cgit v1.2.3