From 8860ad41e07e9467f8a2a0805f9f665f9acbcb7f Mon Sep 17 00:00:00 2001 From: Peter Bridgman Date: Wed, 4 Aug 2021 14:17:51 +0100 Subject: [PATCH] macOS: pass JackMachSemaphore send right directly via shm Previously, JackMachSemaphore would communicate the send right for the semaphore object from the server to the client via a named service regstered via `bootstrap_register`. However, * This is deprecated (see bootstrap.h) - the recommendation is to pass the send right for the port directly, rather than indirecting via the service name and the bootstrap server * This led to #784, wherein old service names were not cleaned up, and the server would abandon attempting to create the 99th service, leading to clients failing to connect after they had connected 98 times on a given boot of the OS. This commit implements the advice from the deprecation notice in bootstrap.h, communicating the send right for the semaphore port directly via the shared memory segment that we used to use to communicate the service name, bypassing the need to register a named service entirely. This resolves #784. --- AUTHORS.rst | 1 + macosx/JackMachSemaphore.h | 12 +++-- macosx/JackMachSemaphore.mm | 99 ++++++++++--------------------------- 3 files changed, 34 insertions(+), 78 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 7266fd00..7b7185ec 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -72,6 +72,7 @@ Nedko Arnaudov Olaf Hering Olivier Humbert Paul Davis +Peter Bridgman Peter L Jones Pieter Palmers Ricardo Crudo diff --git a/macosx/JackMachSemaphore.h b/macosx/JackMachSemaphore.h index 10406847..e97c4486 100644 --- a/macosx/JackMachSemaphore.h +++ b/macosx/JackMachSemaphore.h @@ -39,12 +39,16 @@ class SERVER_EXPORT JackMachSemaphore : public detail::JackSynchro private: semaphore_t fSemaphore; - mach_port_t fBootPort; int fSharedMem; - char* fSharedName; - bool recursiveBootstrapRegister(int counter); + /*! \brief Pointer to shared memory containing semaphore send right port. + * + * If the semaphore has been Allocate()d (in the case of the server) or Connect()ed (in the + * case of the client), and not yet Disconnect()ed or Destroy()ed, a pointer to a shared + * memory segment at which a send right for a semaphore can be found. Otherwise, NULL. + */ + mach_port_t* fSharedSemaphoreSend; protected: @@ -52,7 +56,7 @@ class SERVER_EXPORT JackMachSemaphore : public detail::JackSynchro public: - JackMachSemaphore():JackSynchro(), fSemaphore(0), fBootPort(0), fSharedMem(0), fSharedName(NULL) + JackMachSemaphore():JackSynchro(), fSemaphore(0), fSharedMem(0), fSharedSemaphoreSend(NULL) {} bool Signal(); diff --git a/macosx/JackMachSemaphore.mm b/macosx/JackMachSemaphore.mm index 7b7f4e2e..4eec7267 100644 --- a/macosx/JackMachSemaphore.mm +++ b/macosx/JackMachSemaphore.mm @@ -109,39 +109,6 @@ bool JackMachSemaphore::TimedWait(long usec) return (res == KERN_SUCCESS); } -bool JackMachSemaphore::recursiveBootstrapRegister(int counter) -{ - if (counter == 99) - return false; - - kern_return_t res; - - if ((res = bootstrap_register(fBootPort, fSharedName, fSemaphore)) != KERN_SUCCESS) { - switch (res) { - case BOOTSTRAP_SUCCESS : - break; - - case BOOTSTRAP_NOT_PRIVILEGED : - case BOOTSTRAP_NAME_IN_USE : - case BOOTSTRAP_UNKNOWN_SERVICE : - case BOOTSTRAP_SERVICE_ACTIVE : - // try again with next suffix - snprintf(fSharedName, sizeof(fName), "%s-%d", fName, ++counter); - return recursiveBootstrapRegister(counter); - break; - - default : - jack_log("bootstrap_register() err = %i:%s", res, bootstrap_strerror(res)); - break; - } - - jack_error("Allocate: can't check in mach semaphore name = %s err = %i:%s", fName, res, bootstrap_strerror(res)); - return false; - } - - return true; -} - // Server side : publish the semaphore in the global namespace bool JackMachSemaphore::Allocate(const char* name, const char* server_name, int value) { @@ -149,38 +116,31 @@ bool JackMachSemaphore::Allocate(const char* name, const char* server_name, int mach_port_t task = mach_task_self(); kern_return_t res; - if (fBootPort == 0) { - if ((res = task_get_bootstrap_port(task, &fBootPort)) != KERN_SUCCESS) { - jack_error("Allocate: Can't find bootstrap mach port err = %s", mach_error_string(res)); - return false; - } - } - if ((fSharedMem = shm_open(fName, O_CREAT | O_RDWR, 0777)) < 0) { - jack_error("Allocate: can't check in mach shared name = %s err = %s", fName, strerror(errno)); + jack_error("Allocate: can't open shared memory segment; name = %s err = %s", fName, strerror(errno)); return false; } struct stat st; if (fstat(fSharedMem, &st) != -1 && st.st_size == 0) { - if (ftruncate(fSharedMem, SYNC_MAX_NAME_SIZE+1) != 0) { + if (ftruncate(fSharedMem, sizeof(mach_port_t)) != 0) { jack_error("Allocate: can't set shared memory size in mach shared name = %s err = %s", fName, strerror(errno)); return false; } } - char* const sharedName = (char*)mmap(NULL, SYNC_MAX_NAME_SIZE+1, PROT_READ|PROT_WRITE, MAP_SHARED, fSharedMem, 0); + mach_port_t* const sharedSemaphoreSend = + (mach_port_t*)mmap(NULL, sizeof(mach_port_t), PROT_READ | PROT_WRITE, MAP_SHARED, fSharedMem, 0); - if (sharedName == NULL || sharedName == MAP_FAILED) { - jack_error("Allocate: can't check in mach shared name = %s err = %s", fName, strerror(errno)); + if (sharedSemaphoreSend == NULL || sharedSemaphoreSend == MAP_FAILED) { + jack_error("Allocate: failed to mmap; name = %s err = %s", fName, strerror(errno)); close(fSharedMem); fSharedMem = -1; shm_unlink(fName); return false; } - fSharedName = sharedName; - strcpy(fSharedName, fName); + fSharedSemaphoreSend = sharedSemaphoreSend; if ((res = semaphore_create(task, &fSemaphore, SYNC_POLICY_FIFO, value)) != KERN_SUCCESS) { jack_error("Allocate: can create semaphore err = %i:%s", res, mach_error_string(res)); @@ -188,7 +148,10 @@ bool JackMachSemaphore::Allocate(const char* name, const char* server_name, int } jack_log("JackMachSemaphore::Allocate name = %s", fName); - return recursiveBootstrapRegister(1); + + *fSharedSemaphoreSend = fSemaphore; + + return true; } // Client side : get the published semaphore from server @@ -198,40 +161,28 @@ bool JackMachSemaphore::ConnectInput(const char* name, const char* server_name) kern_return_t res; // Temporary... - if (fSharedName) { + if (fSharedSemaphoreSend) { jack_log("Already connected name = %s", name); return true; } - if (fBootPort == 0) { - if ((res = task_get_bootstrap_port(mach_task_self(), &fBootPort)) != KERN_SUCCESS) { - jack_error("Connect: can't find bootstrap port err = %s", mach_error_string(res)); - return false; - } - } - if ((fSharedMem = shm_open(fName, O_RDWR, 0)) < 0) { - jack_error("Connect: can't connect mach shared name = %s err = %s", fName, strerror(errno)); + jack_error("Connect: can't open shared memory segment; name = %s err = %s", fName, strerror(errno)); return false; } - char* const sharedName = (char*)mmap(NULL, SYNC_MAX_NAME_SIZE+1, PROT_READ|PROT_WRITE, MAP_SHARED, fSharedMem, 0); - - if (sharedName == NULL || sharedName == MAP_FAILED) { - jack_error("Connect: can't connect mach shared name = %s err = %s", fName, strerror(errno)); - close(fSharedMem); - fSharedMem = -1; - return false; - } + mach_port_t* const sharedSemaphoreSend = + (mach_port_t*)mmap(NULL, sizeof(mach_port_t), PROT_READ | PROT_WRITE, MAP_SHARED, fSharedMem, 0); - if ((res = bootstrap_look_up(fBootPort, sharedName, &fSemaphore)) != KERN_SUCCESS) { - jack_error("Connect: can't find mach semaphore name = %s, sname = %s, err = %s", fName, sharedName, bootstrap_strerror(res)); + if (sharedSemaphoreSend == NULL || sharedSemaphoreSend == MAP_FAILED) { + jack_error("Connect: failed to mmap: name = %s err = %s", fName, strerror(errno)); close(fSharedMem); fSharedMem = -1; return false; } - fSharedName = sharedName; + fSharedSemaphoreSend = sharedSemaphoreSend; + fSemaphore = *sharedSemaphoreSend; jack_log("JackMachSemaphore::Connect name = %s ", fName); return true; @@ -254,12 +205,12 @@ bool JackMachSemaphore::Disconnect() fSemaphore = 0; } - if (!fSharedName) { + if (!fSharedSemaphoreSend) { return true; } - munmap(fSharedName, SYNC_MAX_NAME_SIZE+1); - fSharedName = NULL; + munmap(fSharedSemaphoreSend, sizeof(mach_port_t)); + fSharedSemaphoreSend = NULL; close(fSharedMem); fSharedMem = -1; @@ -281,12 +232,12 @@ void JackMachSemaphore::Destroy() jack_error("JackMachSemaphore::Destroy semaphore < 0"); } - if (!fSharedName) { + if (!fSharedSemaphoreSend) { return; } - munmap(fSharedName, SYNC_MAX_NAME_SIZE+1); - fSharedName = NULL; + munmap(fSharedSemaphoreSend, sizeof(mach_port_t)); + fSharedSemaphoreSend = NULL; close(fSharedMem); fSharedMem = -1;