From e18a827ec1075f3f949d49267bf5594d97bcacae Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Thu, 14 Dec 2023 20:54:45 +0100 Subject: [PATCH] Windows: Fix duplicate concurrency_{globalStopSourcePointer,getLocalThreadExecutor} symbols The `export` visibility causes these 2 special symbols to be exported from every binary the concurrency library is linked statically into. Now if we have a DLL exporting it, and the executable too, and the executable is linked implicitly against the DLL (via .lib import library), the linker complains about duplicate symbols. So on Windows, it should generally be exported from a single binary in the whole process. AFAICT, the `dynamicLoad` utility checks the executable's exports only, so relies on that exporting binary being the executable. This can be achieved via extra linker flags for the executable; AFAIK, there's unfortunately no way to un-export specific symbols in the linker command-line (which would alternatively allow preventing the export via linker flags for the DLLs). --- .github/workflows/main.yml | 1 + README.md | 7 ++++- dub.sdl | 3 ++ source/concurrency/signal.d | 38 +++++++++++++++++------- source/concurrency/thread.d | 58 ++++++++++++++++++++++++------------- 5 files changed, 76 insertions(+), 31 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e965e4e..6cd6955 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -5,6 +5,7 @@ jobs: test: name: Dub Tests strategy: + fail-fast: false matrix: os: [ubuntu-latest, windows-latest] dc: [dmd-latest, ldc-latest, dmd-2.098.1, ldc-1.28.1] diff --git a/README.md b/README.md index 95e6928..688c20f 100644 --- a/README.md +++ b/README.md @@ -206,7 +206,12 @@ If you want the termination to happen asynchronously, for instance because the c The concurrency library is designed to work with dynamic libraries. It exports 2 functions that it uses to load important globals and thread-locals from the host process. -The only requirement is that the linker you are using supports `--export-dynamic-symbol` (at least gold, lld do). +On Posix, the only requirement is that the linker you are using supports `--export-dynamic-symbol` (at least gold, lld do). + +On Windows, you need to export the 2 symbols explicitly from the executable to make the whole process (i.e., DLLs with statically linked `concurrency`) use these. This can be achieved by adding according linker flags for the executable, e.g., in `dub.sdl`: +``` +lflags "/EXPORT:concurrency_getLocalThreadExecutor" "/EXPORT:concurrency_globalStopSourcePointer" platform="windows" +``` ## DSemver diff --git a/dub.sdl b/dub.sdl index 4b36060..9e7568d 100644 --- a/dub.sdl +++ b/dub.sdl @@ -17,6 +17,7 @@ configuration "unittest" { dflags "-dip1000" sourcePaths "source" "tests" importPaths "source" "tests" + lflags "/EXPORT:concurrency_getLocalThreadExecutor" "/EXPORT:concurrency_globalStopSourcePointer" platform="windows" } configuration "unittest-release" { dependency "unit-threaded" version="*" @@ -25,6 +26,7 @@ configuration "unittest-release" { dflags "-dip1000" "-g" sourcePaths "source" "tests/ut" importPaths "source" "tests/ut" + lflags "/EXPORT:concurrency_getLocalThreadExecutor" "/EXPORT:concurrency_globalStopSourcePointer" platform="windows" # buildOptions "unittests" "optimize" "inline" buildOptions "unittests" "optimize" } @@ -39,6 +41,7 @@ configuration "unittest-asan" { dflags "-dip1000" "-fsanitize=address" sourcePaths "source" "tests/ut" importPaths "source" "tests/ut" + lflags "/EXPORT:concurrency_getLocalThreadExecutor" "/EXPORT:concurrency_globalStopSourcePointer" platform="windows" # buildOptions "unittests" "optimize" "inline" buildOptions "unittests" "optimize" } diff --git a/source/concurrency/signal.d b/source/concurrency/signal.d index f03b597..97acfb5 100644 --- a/source/concurrency/signal.d +++ b/source/concurrency/signal.d @@ -73,18 +73,36 @@ void setupCtrlCHandler(shared StopSource stopSource) @trusted { private static shared StopSource globalSource; -// we export this function so that dynamic libraries can load it to access -// the host's globalStopSource pointer. -// Otherwise they would access their own local instance. -// should not be called directly by usercode, instead use `globalStopSource`. -export extern(C) -shared(StopSource*) concurrency_globalStopSourcePointer() @safe { - return &globalSource; +// a mixin for OS-specific visibility +private mixin template globalStopSourcePointerImpl() { + // should not be called directly by usercode, call `getGlobalStopSourcePointer` instead + pragma(inline, false) + extern(C) shared(StopSource*) concurrency_globalStopSourcePointer() @safe { + return &globalSource; + } } -private shared(StopSource*) getGlobalStopSourcePointer() @safe { - import concurrency.utils : dynamicLoad; - return dynamicLoad!concurrency_globalStopSourcePointer()(); +// We need to make sure all binaries (executable and shared libraries) in the +// process share a single StopSource instance. +version(Windows) { + // On Windows, the executable can export `concurrency_globalStopSourcePointer` + // explicitly via linker flag `/EXPORT:concurrency_globalStopSourcePointer`. + // DLLs containing `concurrency` use the executable's then, falling back to + // their own definition. + private mixin globalStopSourcePointerImpl; + + private shared(StopSource*) getGlobalStopSourcePointer() @safe { + import concurrency.utils : dynamicLoad; + return dynamicLoad!concurrency_globalStopSourcePointer()(); + } +} else { + // Make sure the `concurrency_globalStopSourcePointer` function gets public + // visibility; coupled with the `--export-dynamic-symbol=…` linker flag in + // dub.sdl, the symbol is then exported as dynamic symbol from every binary + // containing this `concurrency` library, and the dynamic loader uniques the + // symbol across the whole process for us. + export mixin globalStopSourcePointerImpl; + alias getGlobalStopSourcePointer = concurrency_globalStopSourcePointer; } struct SignalHandler { diff --git a/source/concurrency/thread.d b/source/concurrency/thread.d index 2b62c37..7ed8881 100644 --- a/source/concurrency/thread.d +++ b/source/concurrency/thread.d @@ -10,29 +10,47 @@ import core.time : Duration; import concurrency.data.queue.waitable; import concurrency.data.queue.mpsc; -// we export the getLocalThreadExecutor function so that dynamic libraries -// can load it to access the host's localThreadExecutor TLS instance. -// Otherwise they would access their own local instance. -// should not be called directly by usercode, call `silThreadExecutor` instead. -export extern(C) -LocalThreadExecutor concurrency_getLocalThreadExecutor() @safe { - static LocalThreadExecutor localThreadExecutor; - if (localThreadExecutor is null) { - localThreadExecutor = new LocalThreadExecutor(); - } - - return localThreadExecutor; -} +// a mixin for OS-specific visibility +private mixin template getLocalThreadExecutorImpl() { + // should not be called directly by usercode, call `getLocalThreadExecutor` instead + pragma(inline, false) + extern(C) LocalThreadExecutor concurrency_getLocalThreadExecutor() @safe { + static LocalThreadExecutor localThreadExecutor; + if (localThreadExecutor is null) { + localThreadExecutor = new LocalThreadExecutor(); + } -LocalThreadExecutor getLocalThreadExecutor() @trusted { - import concurrency.utils : dynamicLoad; - static LocalThreadExecutor localThreadExecutor; - if (localThreadExecutor is null) { - localThreadExecutor = - dynamicLoad!concurrency_getLocalThreadExecutor()(); + return localThreadExecutor; } +} - return localThreadExecutor; +// We need to make sure all binaries (executable and shared libraries) in the +// process share a single (thread-local) LocalThreadExecutor instance. +version(Windows) { + // On Windows, the executable can export `concurrency_getLocalThreadExecutor` + // explicitly via linker flag `/EXPORT:concurrency_getLocalThreadExecutor`. + // DLLs containing `concurrency` use the executable's then, falling back to + // their own definition. + mixin getLocalThreadExecutorImpl; + + LocalThreadExecutor getLocalThreadExecutor() @trusted { + import concurrency.utils : dynamicLoad; + static LocalThreadExecutor localThreadExecutor; + if (localThreadExecutor is null) { + localThreadExecutor = + dynamicLoad!concurrency_getLocalThreadExecutor()(); + } + + return localThreadExecutor; + } +} else { + // Make sure the `concurrency_getLocalThreadExecutor` function gets public + // visibility; coupled with the `--export-dynamic-symbol=…` linker flag in + // dub.sdl, the symbol is then exported as dynamic symbol from every binary + // containing this `concurrency` library, and the dynamic loader uniques the + // symbol across the whole process for us. + export mixin getLocalThreadExecutorImpl; + alias getLocalThreadExecutor = concurrency_getLocalThreadExecutor; } private struct AddTimer {