From c5a10080678c95c4a0d77e206c67dc3802684934 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 + dub.sdl | 3 ++ source/concurrency/signal.d | 38 +++++++++++++++++------- source/concurrency/thread.d | 58 +++++++++++++++++++++++-------------- 4 files changed, 69 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/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..15696c9 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..9b21d31 100644 --- a/source/concurrency/thread.d +++ b/source/concurrency/thread.d @@ -10,29 +10,45 @@ 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(); + } + + return localThreadExecutor; + } } -LocalThreadExecutor getLocalThreadExecutor() @trusted { - import concurrency.utils : dynamicLoad; - static LocalThreadExecutor localThreadExecutor; - if (localThreadExecutor is null) { - localThreadExecutor = - dynamicLoad!concurrency_getLocalThreadExecutor()(); - } - - 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 {