From 45042beac57c1a7ca35a91fcf15ad41c9b66daa4 Mon Sep 17 00:00:00 2001 From: falkTX Date: Fri, 15 Apr 2022 20:02:58 +0100 Subject: [PATCH] Make JackMachSemaphore more robust, dont use thread_terminate Fixes #841 --- macosx/JackMachSemaphore.mm | 33 ++++++++++++++++++++----------- macosx/JackMachSemaphoreServer.h | 14 ++++++++----- macosx/JackMachSemaphoreServer.mm | 24 +++++++++++++++++++++- macosx/JackMachThread.mm | 5 +++++ 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/macosx/JackMachSemaphore.mm b/macosx/JackMachSemaphore.mm index 1905e04c..c28fe5ab 100644 --- a/macosx/JackMachSemaphore.mm +++ b/macosx/JackMachSemaphore.mm @@ -305,7 +305,7 @@ bool JackMachSemaphore::ConnectInput(const char* client_name, const char* server } 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 @@ bool JackMachSemaphore::Disconnect() // 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 @@ void JackMachSemaphore::Destroy() 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 index 85f3522f..0b2ebeeb 100644 --- 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 index 21f159b0..c562e01e 100644 --- a/macosx/JackMachSemaphoreServer.mm +++ b/macosx/JackMachSemaphoreServer.mm @@ -52,9 +52,14 @@ bool JackMachSemaphoreServer::Execute() { 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 @@ bool JackMachSemaphoreServer::Execute() { 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 index 061c7919..b42de3e7 100644 --- a/macosx/JackMachThread.mm +++ b/macosx/JackMachThread.mm @@ -160,6 +160,8 @@ int JackMachThread::GetParams(jack_native_thread_t thread, UInt64* period, UInt6 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 @@ int JackMachThread::Kill() } else { return -1; } +#else + return JackPosixThread::Kill(); +#endif } int JackMachThread::AcquireRealTime()