Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
macOS: killing a thread will abort the semaphore wait
Signed-off-by: falkTX <falktx@falktx.com>
  • Loading branch information
falkTX committed Jan 29, 2023
1 parent ae9993b commit 79ea074
Showing 1 changed file with 16 additions and 8 deletions.
24 changes: 16 additions & 8 deletions macosx/JackMachSemaphore.mm
Expand Up @@ -90,11 +90,15 @@
return false;
}

kern_return_t res;
if ((res = semaphore_wait(fSemaphore)) != KERN_SUCCESS) {
jack_error("JackMachSemaphore::Wait name = %s err = %s", fName, mach_error_string(res));
kern_return_t res = semaphore_wait(fSemaphore);

// killing a thread will abort the semaphore wait
if (res == KERN_SUCCESS || res == KERN_ABORTED) {
return true;
}
return (res == KERN_SUCCESS);

jack_error("JackMachSemaphore::Wait name = %s err = %s", fName, mach_error_string(res));
return false;
}

bool JackMachSemaphore::TimedWait(long usec)
Expand All @@ -104,15 +108,19 @@
return false;
}

kern_return_t res;
mach_timespec time;
time.tv_sec = usec / 1000000;
time.tv_nsec = (usec % 1000000) * 1000;

if ((res = semaphore_timedwait(fSemaphore, time)) != KERN_SUCCESS) {
jack_error("JackMachSemaphore::TimedWait name = %s usec = %ld err = %s", fName, usec, mach_error_string(res));
kern_return_t res = semaphore_timedwait(fSemaphore, time);

// killing a thread will abort the semaphore wait
if (res == KERN_SUCCESS || res == KERN_ABORTED) {
return true;
}
return (res == KERN_SUCCESS);

jack_error("JackMachSemaphore::TimedWait name = %s usec = %ld err = %s", fName, usec, mach_error_string(res));
return false;
}

/*! \brief Server side: create semaphore and publish IPC primitives to make it accessible.
Expand Down

7 comments on commit 79ea074

@gisogrimm
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure that it is related, but exiting a client hangs with jack 1.9.22 (universal binary installed from jackaudio.org), but not with 1.9.21 (installed via brew) (both on macOS Ventura 13.1). This is independent of the backend (tested with coreaudio and dummy). I will test on monterey later. In both cases I get error messages like

JackMachSemaphore::TimedWait name = ....... err = (os/kern) aborted
SuspendRefNum error

@falkTX
Copy link
Member Author

@falkTX falkTX commented on 79ea074 Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is odd then, because the issue was forcely fixed in f5a0199
the message you pasted shouldnt be logged anymore, since in the commit above the abort case is handled to skip such log.

@gisogrimm
Copy link

@gisogrimm gisogrimm commented on 79ea074 Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see, the client was linked against jack 1.9.21 in both cases, I tested only different server instances. Using 1.9.22 in client and server does not result in problems upon shutdown.

@falkTX
Copy link
Member Author

@falkTX falkTX commented on 79ea074 Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the linking on client side shouldnt make a difference, so not sure what is up..

according to the build logs https://github.com/jackaudio/jack2-releases/actions/runs/4080111352/jobs/7032228884 release is using the correct and expected commit.

this needs a bit more testing

@gisogrimm
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem disappears when starting the client with
DYLD_LIBRARY_PATH=/usr/local/lib
(the place where the 1.9.22 jack was installed). If I don't set the variable, it uses 1.9.21 from homebrew.

@falkTX
Copy link
Member Author

@falkTX falkTX commented on 79ea074 Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, remove the homebrew version then. you shouldnt have 2 jack libs installed system-wide like that, it for sure will lead to issues

@falkTX
Copy link
Member Author

@falkTX falkTX commented on 79ea074 Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or alternatively always use homebrew versions, but then contact them first for support in case of issues, falling back to the jack2 project when indicated by them

Please sign in to comment.