From 14c232bdbfbb16f427632d579430fa1a522f7f73 Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Wed, 21 Sep 2016 15:55:34 +0200
Subject: client API: fix init/destruction race conditions

mp_new_client() blatantly accessed some mutex-protected state outside of
the mutex.

The destruction code is in theory OK, but with changes in the following
commits it'll be a bit hard to guarantee that it stays this way. Add a
simple flag that makes adding new clients impossible, so that having no
clients after shutdown_clients() remains guaranteed.
---
 player/client.c | 18 ++++++++++++++----
 player/client.h |  1 +
 player/main.c   |  8 ++++----
 3 files changed, 19 insertions(+), 8 deletions(-)

(limited to 'player')

diff --git a/player/client.c b/player/client.c
index 5d81f7c93b..93db0bb3d2 100644
--- a/player/client.c
+++ b/player/client.c
@@ -66,7 +66,8 @@ struct mp_client_api {
 
     struct mpv_handle **clients;
     int num_clients;
-    uint64_t event_masks;   // combined events of all clients, or 0 if unknown
+    uint64_t event_masks; // combined events of all clients, or 0 if unknown
+    bool shutting_down; // do not allow new clients
 
     struct mp_custom_protocol *custom_protocols;
     int num_custom_protocols;
@@ -206,8 +207,17 @@ bool mp_client_exists(struct MPContext *mpctx, const char *client_name)
     return r;
 }
 
+void mp_client_enter_shutdown(struct MPContext *mpctx)
+{
+    pthread_mutex_lock(&mpctx->clients->lock);
+    mpctx->clients->shutting_down = true;
+    pthread_mutex_unlock(&mpctx->clients->lock);
+}
+
 struct mpv_handle *mp_new_client(struct mp_client_api *clients, const char *name)
 {
+    pthread_mutex_lock(&clients->lock);
+
     char nname[MAX_CLIENT_NAME];
     for (int n = 1; n < 1000; n++) {
         if (!name)
@@ -222,10 +232,10 @@ struct mpv_handle *mp_new_client(struct mp_client_api *clients, const char *name
         nname[0] = '\0';
     }
 
-    if (!nname[0])
+    if (!nname[0] || clients->shutting_down) {
+        pthread_mutex_unlock(&clients->lock);
         return NULL;
-
-    pthread_mutex_lock(&clients->lock);
+    }
 
     int num_events = 1000;
 
diff --git a/player/client.h b/player/client.h
index e8866225a7..e39d0e676a 100644
--- a/player/client.h
+++ b/player/client.h
@@ -16,6 +16,7 @@ struct mp_log;
 #define MAX_CLIENT_NAME 64
 
 void mp_clients_init(struct MPContext *mpctx);
+void mp_client_enter_shutdown(struct MPContext *mpctx);
 void mp_clients_destroy(struct MPContext *mpctx);
 int mp_clients_num(struct MPContext *mpctx);
 bool mp_clients_all_initialized(struct MPContext *mpctx);
diff --git a/player/main.c b/player/main.c
index b4371ed1fa..c3488fc60c 100644
--- a/player/main.c
+++ b/player/main.c
@@ -152,20 +152,20 @@ void mp_print_version(struct mp_log *log, int always)
 
 static void shutdown_clients(struct MPContext *mpctx)
 {
-    while (mpctx->clients && mp_clients_num(mpctx)) {
+    mp_client_enter_shutdown(mpctx);
+    while (mp_clients_num(mpctx)) {
         mp_client_broadcast_event(mpctx, MPV_EVENT_SHUTDOWN, NULL);
-        mp_dispatch_queue_process(mpctx->dispatch, 0);
         mp_wait_events(mpctx);
     }
 }
 
 void mp_destroy(struct MPContext *mpctx)
 {
+    shutdown_clients(mpctx);
+
     mp_uninit_ipc(mpctx->ipc_ctx);
     mpctx->ipc_ctx = NULL;
 
-    shutdown_clients(mpctx);
-
     uninit_audio_out(mpctx);
     uninit_video_out(mpctx);
 
-- 
cgit v1.2.3