From fff99c726e53808b5b75a89ae66e5e84ab19268e Mon Sep 17 00:00:00 2001 From: pkova Date: Tue, 17 Sep 2024 01:52:51 +0300 Subject: [PATCH 1/4] Fix core sync test deadlock on darwin --- core/sync/futex_darwin.odin | 10 ++++++++-- core/sys/darwin/sync.odin | 7 +++++++ tests/core/sync/test_core_sync.odin | 1 - 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/core/sync/futex_darwin.odin b/core/sync/futex_darwin.odin index daefd6699..3915a414d 100644 --- a/core/sync/futex_darwin.odin +++ b/core/sync/futex_darwin.odin @@ -12,6 +12,7 @@ foreign System { // __ulock_wait is not available on 10.15 // See https://github.com/odin-lang/Odin/issues/1959 __ulock_wait :: proc "c" (operation: u32, addr: rawptr, value: u64, timeout_us: u32) -> c.int --- + __ulock_wait2 :: proc "c" (operation: u32, addr: rawptr, value: u64, timeout_ns: u64, value2: u64) -> c.int --- __ulock_wake :: proc "c" (operation: u32, addr: rawptr, wake_value: u64) -> c.int --- } @@ -52,8 +53,13 @@ _futex_wait_with_timeout :: proc "contextless" (f: ^Futex, expected: u32, durati } } else { - timeout_ns := u32(duration) - s := __ulock_wait(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, f, u64(expected), timeout_ns) + when darwin.ULOCK_WAIT_2_AVAILABLE { + timeout_ns := u64(duration) + s := __ulock_wait2(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, f, u64(expected), timeout_ns, 0) + } else { + timeout_us := u32(duration) + s := __ulock_wait(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, f, u64(expected), timeout_us) + } if s >= 0 { return true } diff --git a/core/sys/darwin/sync.odin b/core/sys/darwin/sync.odin index 121d3edef..e90f30162 100644 --- a/core/sys/darwin/sync.odin +++ b/core/sys/darwin/sync.odin @@ -12,8 +12,15 @@ when ODIN_OS == .Darwin { } else { WAIT_ON_ADDRESS_AVAILABLE :: false } + when ODIN_MINIMUM_OS_VERSION >= 11_00_00 { + ULOCK_WAIT_2_AVAILABLE :: true + } else { + ULOCK_WAIT_2_AVAILABLE :: false + } + } else { WAIT_ON_ADDRESS_AVAILABLE :: false + ULOCK_WAIT_2_AVAILABLE :: false } os_sync_wait_on_address_flag :: enum u32 { diff --git a/tests/core/sync/test_core_sync.odin b/tests/core/sync/test_core_sync.odin index 32c08f935..fdd865686 100644 --- a/tests/core/sync/test_core_sync.odin +++ b/tests/core/sync/test_core_sync.odin @@ -8,7 +8,6 @@ // These tests are temporarily disabled on Darwin, as there is currently a // stall occurring which I cannot debug. -//+build !darwin package test_core_sync import "base:intrinsics" From aa25714d43716a857219d42796ade19de6c5ef70 Mon Sep 17 00:00:00 2001 From: pkova Date: Tue, 17 Sep 2024 02:11:41 +0300 Subject: [PATCH 2/4] Remove comment from core sync tests now that they're fixed --- tests/core/sync/test_core_sync.odin | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/core/sync/test_core_sync.odin b/tests/core/sync/test_core_sync.odin index fdd865686..d6a7a9517 100644 --- a/tests/core/sync/test_core_sync.odin +++ b/tests/core/sync/test_core_sync.odin @@ -4,9 +4,6 @@ // Keep in mind that running with the debug logs uncommented can result in // failures disappearing due to the delay of sending the log message causing // different synchronization patterns. -// -// These tests are temporarily disabled on Darwin, as there is currently a -// stall occurring which I cannot debug. package test_core_sync From 4d6f7dcac01061ee3060c14bb10e27f101998140 Mon Sep 17 00:00:00 2001 From: Pyry Kovanen Date: Tue, 17 Sep 2024 02:21:00 +0300 Subject: [PATCH 3/4] Fix code alignment in futex_darwin.odin Co-authored-by: Feoramund <161657516+Feoramund@users.noreply.github.com> --- core/sync/futex_darwin.odin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/sync/futex_darwin.odin b/core/sync/futex_darwin.odin index 3915a414d..5567be963 100644 --- a/core/sync/futex_darwin.odin +++ b/core/sync/futex_darwin.odin @@ -12,7 +12,7 @@ foreign System { // __ulock_wait is not available on 10.15 // See https://github.com/odin-lang/Odin/issues/1959 __ulock_wait :: proc "c" (operation: u32, addr: rawptr, value: u64, timeout_us: u32) -> c.int --- - __ulock_wait2 :: proc "c" (operation: u32, addr: rawptr, value: u64, timeout_ns: u64, value2: u64) -> c.int --- + __ulock_wait2 :: proc "c" (operation: u32, addr: rawptr, value: u64, timeout_ns: u64, value2: u64) -> c.int --- __ulock_wake :: proc "c" (operation: u32, addr: rawptr, wake_value: u64) -> c.int --- } From 6e0f1cc866e8a566a46ccaf9f14879e7ac344fe2 Mon Sep 17 00:00:00 2001 From: pkova Date: Tue, 17 Sep 2024 02:35:00 +0300 Subject: [PATCH 4/4] Pass microseconds instead of nanoseconds to __ulock_wait --- core/sync/futex_darwin.odin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/sync/futex_darwin.odin b/core/sync/futex_darwin.odin index 5567be963..32fdb1552 100644 --- a/core/sync/futex_darwin.odin +++ b/core/sync/futex_darwin.odin @@ -57,7 +57,7 @@ _futex_wait_with_timeout :: proc "contextless" (f: ^Futex, expected: u32, durati timeout_ns := u64(duration) s := __ulock_wait2(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, f, u64(expected), timeout_ns, 0) } else { - timeout_us := u32(duration) + timeout_us := u32(duration) * 1000 s := __ulock_wait(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, f, u64(expected), timeout_us) } if s >= 0 {