Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dllimport/dllexport in public jack headers #793

Closed
wants to merge 29 commits into from

Conversation

amurzeau
Copy link
Contributor

I've finally come-up with a maybe more "radical" solution.

CFLAGS and CXXFLAGS are modified to add a global BUILDING_JACK define that choose between dllexport and dllimport.

When building jack, symbols are declared using dllexport.
When headers are used by an application, symbols will be declared using dllimport.

Fixes: #792

falkTX and others added 28 commits July 15, 2021 07:29
Signed-off-by: falkTX <[email protected]>
Signed-off-by: falkTX <[email protected]>
It uses c++ sources and runtime therefore its best to use c++ compiler
to build it so it can find the correct runtime, cross compiling with
clang fails

x86_64-yoe-linux-ld: example-clients/simdtests.cpp.28.o: undefined reference to symbol '__cxa_call_unexpected@@CXXABI_1.3'

Signed-off-by: Khem Raj <[email protected]>
Signed-off-by: falkTX <[email protected]>
Signed-off-by: falkTX <[email protected]>
Signed-off-by: falkTX <[email protected]>
Signed-off-by: falkTX <[email protected]>
Signed-off-by: falkTX <[email protected]>
)

* macOS: Pass JackMachSemaphore send right via mach_msg IPC

Previously, JackMachSemaphore would communicate the send right for the
semaphore object from the server to a client via a named service
registered via `bootstrap_register`. However, to do this, it would
register the semaphore's port as the service port directly.

In theory this ought to be fine, however in practice, macOS `launchd`,
which provides the `bootstrap_register` interface, does not correctly
detect when such a port becomes dead, and incorrectly believes that the
service that it provides is forever alive, even past the end of the
`jackd` process' (and therefore the semaphore's) existence. This seems
to be *specific* to semaphore ports, as `launchd` is expecting a
standard IPC port, owned by the task, not the kernel. This prevents
`jackd` from later registering another service with the same name, as
`launchd` rejects the registration as conflicting with an active service.

To get around this, `jackd` previously added a counter to the end of the
named service registrations, allowing old services to remain in the
system until the end of the session. To prevent things getting out of
hand, this was capped at 98 service registrations for a given semaphore
name. This led to jackaudio#784, in which running a client for the 99th time
resulted in the semaphore creation failing and the client failing to
connect.

As `launchd` outlives multiple runs of `jackd`, this situation persisted
across restarts of `jackd`, requiring a restart of the user's session
(i.e. a reboot) to fix.

An initial attempt at fixing this (see jackaudio#785) tried passing the port
rights directly via shared memory, however mach is too clever for us and
foils that plan by having port names be looked up in a per-task table
(sensible when you think about it).

In this commit, we use mach IPC messages to transfer the send right for
the semaphore from the server to the client. By registering a standard
IPC port with the bootstrap server, the service registrations are
correctly torn down when the ports are destroyed.

It works something like this:

* Server creates IPC port and registers it globally via `bootstrap_register`
* Server listens on IPC port for messages
* Client looks up IPC port via `bootstrap_look_up`
* Client sends it a message
* Server replies with a message containing a send right to the
semaphore's port
* Client is then free to use the semaphore port as before.

This resolves jackaudio#784.

* Improve error handling

* Add myself to Authors
@amurzeau
Copy link
Contributor Author

I've tried to compile qjackctl against that version of Jack (from CI) and it worked fine.
Also, running qjackctl (compiled with MSVC) with that Jack version is working too (after minor changes to qjackctl which was using free instead of jack_free).

The qjackctl compiled with MSVC is also working fine with official versions of Jack.

@falkTX
Copy link
Member

falkTX commented Aug 21, 2021

Glad that you found something that works, but BUILDING_JACK macro is not needed.
As I said on the other ticket, the common/jack/* headers are not used when building jack2, so this definition makes nothing. you can set JACK_CLIENT_API_EXPORT to always be dllimport, it should build and work just the same

@falkTX
Copy link
Member

falkTX commented Aug 21, 2021

I meant to say JACK_CLIENT_API_EXPORT as dllimport

@amurzeau
Copy link
Contributor Author

amurzeau commented Aug 21, 2021

I've tried that, but it failed the build: https://github.com/amurzeau/jack2/actions/runs/1152225544
In this build, I've set an equivalent of BUILDING_JACK only for clientlib and serverlib, but the build failed at jacknet as I said here: #792 (comment)

Here is a diff between the failed build linked above in my comment and the current PR state:
https://github.com/amurzeau/jack2/compare/2b164579c44684f4ee3a74007def21971740cf09..use-dllimport-dllexport

@falkTX
Copy link
Member

falkTX commented Aug 21, 2021

hmm so maybe it is just the ring buffer that needs some attention.
you should be able to define the ringbuffer functions instead of including common/jack/ringbuffer.h, the same way that the code sets up the rest of the API calls to export.

@amurzeau
Copy link
Contributor Author

When greping for includes of jack, I find several occurrences (I've removed occurrences from common/jack, tools, tests and example clients).
I will try to see if it is sufficient. Only jacknet seems impacted, dbus is probably never compiled on Windows.

./common/JackMetadata.h:#include <jack/uuid.h>
./common/JackWeakAPI.c:#include <jack/jack.h>
./common/JackWeakAPI.c:#include <jack/session.h>
./common/JackWeakAPI.c:#include <jack/thread.h>
./common/JackWeakAPI.c:#include <jack/midiport.h>
./common/netjack.c:#include <jack/types.h>
./common/netjack.c:#include "jack/jslist.h"
./common/netjack.h:#include <jack/types.h>
./common/netjack.h:#include <jack/jack.h>
./common/netjack.h:#include <jack/transport.h>
./common/netjack.h:#include "jack/jslist.h"
./common/netjack_packet.c:#include <jack/types.h>
./common/netjack_packet.h:#include <jack/jack.h>
./common/netjack_packet.h:#include <jack/types.h>
./common/netjack_packet.h:#include <jack/jslist.h>
./common/netjack_packet.h:#include <jack/midiport.h>
./dbus/audio_reserve.c:#include "jack/control.h"
./dbus/controller_iface_session_manager.c:#include "jack/session.h"
./dbus/controller_iface_session_manager.c:#include "jack/control.h"
./dbus/controller_internal.h:#include "jack/control.h"
./dbus/controller_internal.h:#include "jack/jack.h"
./dbus/controller_internal.h:#include "jack/session.h"
./dbus/jackdbus.c:#include "jack/jack.h"
./dbus/jackdbus.c:#include "jack/jslist.h"
./dbus/jackdbus.c:#include "jack/control.h"
./dbus/params.h:#include "jack/control.h"
./dbus/reserve.c:#include "jack/control.h"
./dbus/sigsegv.c:#include "jack/control.h"
./dbus/xml_nop.c:#include <jack/driver.h>
./dbus/xml_nop.c:#include <jack/engine.h>
./linux/alsa/alsa_midi_impl.h:#include <jack/messagebuffer.h>
./linux/firewire/ffado_driver.h://#include <jack/midiport.h>

@amurzeau
Copy link
Contributor Author

amurzeau commented Aug 21, 2021

I've tried to fix ringbuffer, but clientlib use jack/uuid.h header: https://github.com/amurzeau/jack2/runs/3387122466?check_suite_focus=true

I think that to make the build work fine, there should be no #include to jack header at all in the above list.

But anyway, I find that using a macro to switch between dllimport and dllexport has the benefit of avoiding duplicate code (like I've done with ringbuffer.h) and make the whole thing more maintainable than other solutions.

When building jack, symbols are declared using dllexport.
When headers are used by an application, symbols will be declared using
dllimport.

Fixes: jackaudio#792
@amurzeau
Copy link
Contributor Author

Closing as I'm not putting more effort on it, and using MinGW is a solution.
I still think that using macros is the way to go to avoid too much code change, if someday this is pushed again.

@amurzeau amurzeau closed this Aug 27, 2022
theartful added a commit to theartful/synfig that referenced this pull request Sep 10, 2022
jackaudio does not play well with msvc, since almost all of the symbols
are not exported
see: jackaudio/jack2#792
     jackaudio/jack2#793
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't link with Jack when using MSVC to build application
4 participants