Make JackMachSemaphore more robust, dont use thread_terminate

Fixes #841
jackaudio · Apr 15, 2022 · 45042be · 45042be
1 parent 5aa5861
commit 45042be
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 18 deletions.
diff --git a/macosx/JackMachSemaphore.mm b/macosx/JackMachSemaphore.mm
@@ -305,7 +305,7 @@
     } else {
         fSemaphore = msg.hdr.msgh_remote_port;
 
-        jack_log("JackMachSemaphore::Connect: OK, name = %s ", fName);
+        jack_log("JackMachSemaphore::Connect: OK, name = %s", fName);
         return true;
     }
 }
@@ -353,17 +353,22 @@
 // Server side : destroy the JackGlobals
 void JackMachSemaphore::Destroy()
 {
+    const mach_port_t task = mach_task_self();
     kern_return_t res;
-    mach_port_t task = mach_task_self();
 
     if (fSemaphore == MACH_PORT_NULL) {
         jack_error("JackMachSemaphore::Destroy semaphore is MACH_PORT_NULL; already destroyed?");
         return;
     }
 
+    if (fSemServer && fSemServer->Invalidate()) {
+        fServicePort = MACH_PORT_NULL;
+        fSemaphore = MACH_PORT_NULL;
+    }
+
     if (fThreadSemServer) {
-        if (fThreadSemServer->Kill() < 0) {
-            jack_error("JackMachSemaphore::Destroy failed to kill semaphore server thread...");
+        if (fThreadSemServer->Stop() < 0) {
+            jack_error("JackMachSemaphore::Destroy failed to stop semaphore server thread...");
             // Oh dear. How sad. Never mind.
         }
 
@@ -378,16 +383,20 @@
         delete server;
     }
 
-    if ((res = mach_port_destroy(task, fServicePort)) != KERN_SUCCESS) {
-        jack_mach_error(res, "failed to destroy IPC port");
-    } else {
-        fServicePort = MACH_PORT_NULL;
+    if (fServicePort != MACH_PORT_NULL) {
+        if ((res = mach_port_destroy(task, fServicePort)) != KERN_SUCCESS) {
+            jack_mach_error(res, "failed to destroy IPC port");
+        } else {
+            fServicePort = MACH_PORT_NULL;
+        }
     }
 
-    if ((res = semaphore_destroy(mach_task_self(), fSemaphore)) != KERN_SUCCESS) {
-        jack_mach_error(res, "failed to destroy semaphore");
-    } else {
-        fSemaphore = MACH_PORT_NULL;
+    if (fSemaphore != MACH_PORT_NULL) {
+        if ((res = semaphore_destroy(mach_task_self(), fSemaphore)) != KERN_SUCCESS) {
+            jack_mach_error(res, "failed to destroy semaphore");
+        } else {
+            fSemaphore = MACH_PORT_NULL;
+        }
     }
 
     jack_log("JackMachSemaphore::Destroy: OK, name = %s", fName);

diff --git a/macosx/JackMachSemaphoreServer.h b/macosx/JackMachSemaphoreServer.h
@@ -34,21 +34,25 @@ class SERVER_EXPORT JackMachSemaphoreServer : public JackRunnableInterface
 {
     private:
         /*! \brief The semaphore send right that will be dispatched to clients. */
-        semaphore_t fSemaphore;
+        const semaphore_t fSemaphore;
 
         /*! \brief The port on which we will listen for IPC messages. */
-        mach_port_t fServerReceive;
+        const mach_port_t fServerReceive;
 
         /*! \brief A pointer to a null-terminated string buffer that will be read to obtain the
          * server name for reporting purposes. Not managed at all by this type. */
-        char* fName;
+        const char* const fName;
+
+        /*! \brief Whether thread should keep running. */
+        bool fRunning;
 
     public:
-        JackMachSemaphoreServer(semaphore_t semaphore, mach_port_t server_recv, char* name):
-            fSemaphore(semaphore), fServerReceive(server_recv), fName(name)
+        JackMachSemaphoreServer(semaphore_t semaphore, mach_port_t server_recv, const char* name):
+            fSemaphore(semaphore), fServerReceive(server_recv), fName(name), fRunning(true)
         {}
 
         bool Execute() override;
+        bool Invalidate();
 };
 
 } // end of namespace

diff --git a/macosx/JackMachSemaphoreServer.mm b/macosx/JackMachSemaphoreServer.mm
@@ -52,9 +52,14 @@
         MACH_PORT_NULL
     );
 
+    // this error is expected when deleting ports, we get notified that they somehow changed
+    if (recv_err == MACH_RCV_PORT_CHANGED) {
+        return fRunning;
+    }
+
     if (recv_err != MACH_MSG_SUCCESS) {
         jack_mach_error(recv_err, "receive error");
-        return true; // Continue processing more connections
+        return fRunning; // Continue processing more connections
     }
 
     /* We're going to reuse the message struct that we received the message into to send a reply.
@@ -81,6 +86,23 @@
         jack_mach_error(send_err, "send error");
     }
 
+    return fRunning;
+}
+
+bool JackMachSemaphoreServer::Invalidate() {
+    fRunning = false;
+
+    const mach_port_t task = mach_task_self();
+    kern_return_t res;
+
+    if ((res = mach_port_destroy(task, fServerReceive)) != KERN_SUCCESS) {
+        jack_mach_error(res, "failed to destroy IPC port");
+    }
+
+    if ((res = semaphore_destroy(task, fSemaphore)) != KERN_SUCCESS) {
+        jack_mach_error(res, "failed to destroy semaphore");
+    }
+
     return true;
 }
 

diff --git a/macosx/JackMachThread.mm b/macosx/JackMachThread.mm
@@ -160,6 +160,8 @@
 
 int JackMachThread::Kill()
 {
+#if 0
+    // NOTE: starting macOS 12, this code no longer works
     if (fThread != (jack_native_thread_t)NULL)  { // If thread has been started
         jack_log("JackMachThread::Kill");
         mach_port_t machThread = pthread_mach_thread_np(fThread);
@@ -170,6 +172,9 @@
     } else {
         return -1;
     }
+#else
+    return JackPosixThread::Kill();
+#endif
 }
 
 int JackMachThread::AcquireRealTime()