From 9c72b97e73bc1e6ff29f97850a5237152a5d5771 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Fri, 31 Mar 2023 10:52:36 -0400 Subject: [PATCH 1/9] Explicitly specify the calling convention for PINIT_ONCE_FN --- tests/generic_win_port.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/generic_win_port.c b/tests/generic_win_port.c index ba685a18b..3a4fb2034 100644 --- a/tests/generic_win_port.c +++ b/tests/generic_win_port.c @@ -188,7 +188,7 @@ gettimeofday(struct timeval *tp, void *tzp) typedef void (WINAPI *QueryUnbiasedInterruptTimePreciseT)(PULONGLONG); static QueryUnbiasedInterruptTimePreciseT QueryUnbiasedInterruptTimePrecisePtr; -static BOOL +static BOOL WINAPI mach_absolute_time_init(PINIT_ONCE InitOnce, PVOID Parameter, PVOID *lpContext) { // QueryUnbiasedInterruptTimePrecise() is declared in the Windows headers From 1848afe3064b686e042b4873e0ac13f431654180 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Fri, 31 Mar 2023 09:10:46 -0700 Subject: [PATCH 2/9] build: slightly improve the build for libdispatch Avoid polluting the build directory a small amount given that we can use `-fmodule-map-file=` for the C/C++ build of libdispatch. Unfortunately, for the Swift build, we need to have the file copied over due to the umbrella header resolution. Hopefully this reduces some of the race conditions that we have seen in the build. Thanks to @dgregor for reminding me of the flag! --- .gitignore | 1 - CMakeLists.txt | 22 ++++------------------ src/swift/CMakeLists.txt | 14 +++++++++++++- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index aa26514c9..e0873a153 100644 --- a/.gitignore +++ b/.gitignore @@ -30,4 +30,3 @@ configure libtool .dirstamp /dispatch/module.modulemap -/private/module.modulemap diff --git a/CMakeLists.txt b/CMakeLists.txt index 2bef26395..57a37d3b5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -261,26 +261,12 @@ endif() if(CMAKE_SYSTEM_NAME STREQUAL Darwin) - add_custom_command(OUTPUT - "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - "${PROJECT_SOURCE_DIR}/private/module.modulemap" - COMMAND - ${CMAKE_COMMAND} -E copy_if_different "${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap" "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - COMMAND - ${CMAKE_COMMAND} -E copy_if_different "${PROJECT_SOURCE_DIR}/private/darwin/module.modulemap" "${PROJECT_SOURCE_DIR}/private/module.modulemap") + add_compile_options($<$,$>:-fmodule-map-file=${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap> + $<$,$>:-fmodule-map-file=${PROJECT_SOURCE_DIR}/private/darwin/module.modulemap>) else() - add_custom_command(OUTPUT - "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - "${PROJECT_SOURCE_DIR}/private/module.modulemap" - COMMAND - ${CMAKE_COMMAND} -E copy_if_different "${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap" "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - COMMAND - ${CMAKE_COMMAND} -E copy_if_different "${PROJECT_SOURCE_DIR}/private/generic/module.modulemap" "${PROJECT_SOURCE_DIR}/private/module.modulemap") + add_compile_options($<$,$>:-fmodule-map-file=${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap> + $<$,$>:-fmodule-map-file=${PROJECT_SOURCE_DIR}/private/generic/module.modulemap>) endif() -add_custom_target(module-maps ALL - DEPENDS - "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - "${PROJECT_SOURCE_DIR}/private/module.modulemap") configure_file("${PROJECT_SOURCE_DIR}/cmake/config.h.in" "${PROJECT_BINARY_DIR}/config/config_ac.h") diff --git a/src/swift/CMakeLists.txt b/src/swift/CMakeLists.txt index c26d00d02..317faed2a 100644 --- a/src/swift/CMakeLists.txt +++ b/src/swift/CMakeLists.txt @@ -18,6 +18,18 @@ target_include_directories(DispatchStubs PRIVATE set_target_properties(DispatchStubs PROPERTIES POSITION_INDEPENDENT_CODE YES) + +if(CMAKE_SYSTEM_NAME STREQUAL Darwin) + add_custom_command(OUTPUT ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) +else() + add_custom_command(OUTPUT ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) +endif() +add_custom_target(module-map ALL + DEPENDS ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) + + add_library(swiftDispatch Block.swift Data.swift @@ -42,7 +54,7 @@ target_link_libraries(swiftDispatch PRIVATE BlocksRuntime::BlocksRuntime) target_link_libraries(swiftDispatch PUBLIC dispatch) -add_dependencies(swiftDispatch module-maps) +add_dependencies(swiftDispatch module-map) get_swift_host_arch(swift_arch) install(FILES From ed909bb3a015d151cd8a800fc1a05581c7faaa29 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Fri, 31 Mar 2023 10:57:37 -0700 Subject: [PATCH 3/9] dispatch: attempt to use a VFS overlay Use a VFS overlay to avoid polluting the source tree. --- .gitignore | 1 - dispatch/CMakeLists.txt | 22 ++++++++++------------ dispatch/dispatch-vfs.yaml.in | 11 +++++++++++ src/swift/CMakeLists.txt | 15 ++------------- 4 files changed, 23 insertions(+), 26 deletions(-) create mode 100644 dispatch/dispatch-vfs.yaml.in diff --git a/.gitignore b/.gitignore index e0873a153..1bf54ca69 100644 --- a/.gitignore +++ b/.gitignore @@ -29,4 +29,3 @@ config configure libtool .dirstamp -/dispatch/module.modulemap diff --git a/dispatch/CMakeLists.txt b/dispatch/CMakeLists.txt index 7f68ed381..352915d91 100644 --- a/dispatch/CMakeLists.txt +++ b/dispatch/CMakeLists.txt @@ -1,4 +1,13 @@ +if(CMAKE_SYSTEM_NAME STREQUAL Darwin) + set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap) +else() + set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap) +endif() +configure_file(dispatch-vfs.yaml.in + ${CMAKE_BINARY_DIR}/dispatch-vfs-overlay.yaml + @ONLY) + install(FILES base.h block.h @@ -16,19 +25,8 @@ install(FILES DESTINATION "${INSTALL_DISPATCH_HEADERS_DIR}") if(ENABLE_SWIFT) - set(base_dir "${CMAKE_CURRENT_SOURCE_DIR}") - if(NOT BUILD_SHARED_LIBS) - set(base_dir "${CMAKE_CURRENT_SOURCE_DIR}/generic_static") - endif() - - get_filename_component( - MODULE_MAP - module.modulemap - REALPATH - BASE_DIR "${base_dir}") - install(FILES - ${MODULE_MAP} + ${DISPATCH_MODULE_MAP} DESTINATION "${INSTALL_DISPATCH_HEADERS_DIR}") endif() diff --git a/dispatch/dispatch-vfs.yaml.in b/dispatch/dispatch-vfs.yaml.in new file mode 100644 index 000000000..9416dda81 --- /dev/null +++ b/dispatch/dispatch-vfs.yaml.in @@ -0,0 +1,11 @@ +--- +version: 0 +case-sensitive: false +use-external-names: false +roots: + - name: "@CMAKE_CURRENT_SOURCE_DIR@" + type: directory + contents: + - name: module.modulemap + type: file + external-contents: "@DISPATCH_MODULE_MAP@" diff --git a/src/swift/CMakeLists.txt b/src/swift/CMakeLists.txt index 317faed2a..c073e136d 100644 --- a/src/swift/CMakeLists.txt +++ b/src/swift/CMakeLists.txt @@ -18,18 +18,6 @@ target_include_directories(DispatchStubs PRIVATE set_target_properties(DispatchStubs PROPERTIES POSITION_INDEPENDENT_CODE YES) - -if(CMAKE_SYSTEM_NAME STREQUAL Darwin) - add_custom_command(OUTPUT ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) -else() - add_custom_command(OUTPUT ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) -endif() -add_custom_target(module-map ALL - DEPENDS ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) - - add_library(swiftDispatch Block.swift Data.swift @@ -45,6 +33,8 @@ target_compile_options(swiftDispatch PRIVATE "SHELL:-Xcc -fmodule-map-file=${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" "SHELL:-Xcc -I${PROJECT_SOURCE_DIR}" "SHELL:-Xcc -I${PROJECT_SOURCE_DIR}/src/swift/shims") +target_compile_options(swiftDispatch PUBLIC + "SHELL:-vfsoverlay ${CMAKE_BINARY_DIR}/dispatch-vfs-overlay.yaml") set_target_properties(swiftDispatch PROPERTIES Swift_MODULE_NAME Dispatch Swift_MODULE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/swift @@ -54,7 +44,6 @@ target_link_libraries(swiftDispatch PRIVATE BlocksRuntime::BlocksRuntime) target_link_libraries(swiftDispatch PUBLIC dispatch) -add_dependencies(swiftDispatch module-map) get_swift_host_arch(swift_arch) install(FILES From 29ebbaabe744881d7789b94ff565b6e9955829e1 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Fri, 21 Apr 2023 12:30:38 +0100 Subject: [PATCH 4/9] Mark `DispatchTimeInterval` as `Sendable` This is an `enum` with associated values of `Int` type that are all `Sendable`, so should be safe mark the whole `enum` as such. Resolves https://github.com/apple/swift-corelibs-libdispatch/issues/787. --- src/swift/Time.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/swift/Time.swift b/src/swift/Time.swift index e1f2efc7d..f426bb319 100644 --- a/src/swift/Time.swift +++ b/src/swift/Time.swift @@ -165,7 +165,7 @@ private func toInt64Clamped(_ value: Double) -> Int64 { /// let t1 = DispatchTimeInterval.seconds(Int.max) /// let t2 = DispatchTimeInterval.milliseconds(Int.max) /// let result = t1 == t2 // true -public enum DispatchTimeInterval: Equatable { +public enum DispatchTimeInterval: Equatable, Sendable { case seconds(Int) case milliseconds(Int) case microseconds(Int) From 7c19a4fe839e224030acaf51d655b20202563767 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 25 Apr 2023 07:50:08 +0900 Subject: [PATCH 5/9] DispatchWallTime and Time are also sendable --- src/swift/Time.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/swift/Time.swift b/src/swift/Time.swift index f426bb319..bc93d5e72 100644 --- a/src/swift/Time.swift +++ b/src/swift/Time.swift @@ -16,7 +16,7 @@ import CDispatch -public struct DispatchTime : Comparable { +public struct DispatchTime : Comparable, Sendable { #if HAVE_MACH private static let timebaseInfo: mach_timebase_info_data_t = { var info = mach_timebase_info_data_t(numer: 1, denom: 1) @@ -96,7 +96,7 @@ extension DispatchTime { } } -public struct DispatchWallTime : Comparable { +public struct DispatchWallTime : Comparable, Sendable { public let rawValue: dispatch_time_t public static func now() -> DispatchWallTime { From 966ff45288009272042fb375ed4f73fe387b05a0 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Tue, 23 May 2023 09:56:41 -0700 Subject: [PATCH 6/9] build: add a link against AdvAPI32 on Windows The `WaitChain*` functions require linking against AdvAPI32. --- src/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ac6a9aa09..811e131e0 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -141,6 +141,7 @@ target_link_libraries(dispatch PUBLIC BlocksRuntime::BlocksRuntime) if(CMAKE_SYSTEM_NAME STREQUAL Windows) target_link_libraries(dispatch PRIVATE + AdvAPI32 ShLwApi WS2_32 WinMM From df7fa52b7b5c5464602606403dd6e6ac1eba03ed Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Tue, 23 May 2023 14:07:43 -0700 Subject: [PATCH 7/9] shims: fix a subtle bug in semaphore initialisation on Windows This function is the initializer for the semaphore. The seamphore storage itself may be stack allocated (or heap allocated) but without guarantee of 0-initialisation. As a result, the subsequent CAS for the atomic replacement will fail silently, leaving the previously non-zero value in place, indicating that the value is a valid handle. This would fail randomly and would ultimately result in a crash in the `CloseHandle` call associated with the clean up. This issue was identified by SwiftLint on Windows. --- src/shims/lock.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/shims/lock.c b/src/shims/lock.c index 62fa6f385..88fb8f8b6 100644 --- a/src/shims/lock.c +++ b/src/shims/lock.c @@ -266,6 +266,7 @@ void _dispatch_sema4_init(_dispatch_sema4_t *sema, int policy DISPATCH_UNUSED) // lazily allocate the semaphore port + os_atomic_cmpxchg(sema, *sema, 0, relaxed); while (!dispatch_assume(tmp = CreateSemaphore(NULL, 0, LONG_MAX, NULL))) { _dispatch_temporary_resource_shortage(); } From a0715dc402182ffa26f8c3ac9fb975f00db8423d Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Fri, 2 Jun 2023 08:34:54 -0700 Subject: [PATCH 8/9] dispatch: install the correct modulemap when building static There is a subtle difference between libdispatch built dynamically and statically: DispatchStubs. We erroneously emit ObjC runtime calls into the build and the stubs provides a shim to provide a definition on non-ObjC runtime enabled targets. When built dynamically, this is internalised and distributed as part of dispatch.dll/libdispatch.so, however when built statically, the consumer is responsible for linking against DispatchStubs. The modulemap reflects this and we need to ensure that we correctly provide that modulemap. Thanks to @MaxDesiatov for the help with debugging and testing a fix for this! Fixes: rdar://23335318 --- dispatch/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dispatch/CMakeLists.txt b/dispatch/CMakeLists.txt index 352915d91..a7f5fc306 100644 --- a/dispatch/CMakeLists.txt +++ b/dispatch/CMakeLists.txt @@ -1,8 +1,10 @@ if(CMAKE_SYSTEM_NAME STREQUAL Darwin) set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap) -else() +elseif(BUILD_SHARED_LIBS) set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap) +else() + set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/generic_static/module.modulemap) endif() configure_file(dispatch-vfs.yaml.in ${CMAKE_BINARY_DIR}/dispatch-vfs-overlay.yaml From 0c38954e0033f4ef112fc2b6d8244c798dce30da Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Wed, 14 Jun 2023 15:44:39 -0400 Subject: [PATCH 9/9] Clamp the length for WriteFile calls on win32 pipes. --- src/io.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/io.c b/src/io.c index f853d9bbf..e31e28c82 100644 --- a/src/io.c +++ b/src/io.c @@ -2510,6 +2510,23 @@ _dispatch_operation_perform(dispatch_operation_t op) } bSuccess = TRUE; } else if (GetFileType(hFile) == FILE_TYPE_PIPE) { + // WriteFile with more bytes than are available in the + // buffer of a NOWAIT pipe will immediately return 0, + // so clamp our requested write length to make progress. + IO_STATUS_BLOCK iosb; + FILE_PIPE_LOCAL_INFORMATION fpli; + NTSTATUS status = _dispatch_NtQueryInformationFile(hFile, + &iosb, &fpli, sizeof(fpli), FilePipeLocalInformation); + if (NT_SUCCESS(status)) { + // WriteQuotaAvailable is unreliable in the presence + // of a blocking reader, when it can return zero, so only + // account for it otherwise + if (fpli.WriteQuotaAvailable > 0) { + len = MIN(len, fpli.WriteQuotaAvailable); + } + len = MIN(len, fpli.OutboundQuota); + } + OVERLAPPED ovlOverlapped = {}; bSuccess = WriteFile(hFile, buf, (DWORD)len, (LPDWORD)&processed, &ovlOverlapped);