From aa6d23ca4a5bb15fc8611f9a132c79d3dd0963da Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 20 Mar 2023 15:46:12 +0100 Subject: [PATCH 1/4] Remove windows-sys dependency --- core/Cargo.toml | 8 ----- core/src/thread_parker/windows/bindings.rs | 32 +++++++++++++++++ core/src/thread_parker/windows/keyed_event.rs | 10 ++---- core/src/thread_parker/windows/mod.rs | 21 ++--------- core/src/thread_parker/windows/waitaddress.rs | 35 +++++++------------ 5 files changed, 49 insertions(+), 57 deletions(-) create mode 100644 core/src/thread_parker/windows/bindings.rs diff --git a/core/Cargo.toml b/core/Cargo.toml index c1fd5305..61ddbf94 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -22,14 +22,6 @@ libc = "0.2.95" [target.'cfg(target_os = "redox")'.dependencies] redox_syscall = "0.2.8" -[target.'cfg(windows)'.dependencies] -windows-sys = { version = "0.45.0", features = [ - "Win32_Foundation", - "Win32_System_LibraryLoader", - "Win32_System_SystemServices", - "Win32_System_WindowsProgramming", -] } - [features] nightly = [] deadlock_detection = ["petgraph", "thread-id", "backtrace"] diff --git a/core/src/thread_parker/windows/bindings.rs b/core/src/thread_parker/windows/bindings.rs new file mode 100644 index 00000000..996eb4e6 --- /dev/null +++ b/core/src/thread_parker/windows/bindings.rs @@ -0,0 +1,32 @@ +pub const INFINITE: u32 = 4294967295; +pub const ERROR_TIMEOUT: u32 = 1460; +pub const GENERIC_READ: u32 = 2147483648; +pub const GENERIC_WRITE: u32 = 1073741824; +pub const STATUS_SUCCESS: i32 = 0 as _; +pub const STATUS_TIMEOUT: i32 = 258 as _; + +pub type HANDLE = isize; +pub type HINSTANCE = isize; +pub type BOOL = i32; +pub type BOOLEAN = u8; +pub type NTSTATUS = i32; +pub type FARPROC = Option isize>; +pub type WaitOnAddress = unsafe extern "system" fn( + Address: *const std::ffi::c_void, + CompareAddress: *const std::ffi::c_void, + AddressSize: usize, + dwMilliseconds: u32, +) -> BOOL; +pub type WakeByAddressSingle = unsafe extern "system" fn(Address: *const std::ffi::c_void); + +#[link(name = "kernel32")] +extern "system" { + pub fn GetLastError() -> u32; + pub fn CloseHandle(hObject: HANDLE) -> BOOL; + + pub fn GetModuleHandleA(lpModuleName: *const u8) -> HINSTANCE; + pub fn GetProcAddress(hModule: HINSTANCE, lpProcName: *const u8) -> FARPROC; + + pub fn Sleep(dwMilliseconds: u32); +} + diff --git a/core/src/thread_parker/windows/keyed_event.rs b/core/src/thread_parker/windows/keyed_event.rs index 302bab58..84a71d03 100644 --- a/core/src/thread_parker/windows/keyed_event.rs +++ b/core/src/thread_parker/windows/keyed_event.rs @@ -13,18 +13,12 @@ use core::{ use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Instant; -use windows_sys::Win32::{ - Foundation::{CloseHandle, BOOLEAN, HANDLE, NTSTATUS, STATUS_SUCCESS, STATUS_TIMEOUT}, - System::{ - LibraryLoader::{GetModuleHandleA, GetProcAddress}, - SystemServices::{GENERIC_READ, GENERIC_WRITE}, - }, -}; - const STATE_UNPARKED: usize = 0; const STATE_PARKED: usize = 1; const STATE_TIMED_OUT: usize = 2; +use super::bindings::*; + #[allow(non_snake_case)] pub struct KeyedEvent { handle: HANDLE, diff --git a/core/src/thread_parker/windows/mod.rs b/core/src/thread_parker/windows/mod.rs index 1f5ed237..876d89dc 100644 --- a/core/src/thread_parker/windows/mod.rs +++ b/core/src/thread_parker/windows/mod.rs @@ -11,6 +11,7 @@ use core::{ }; use std::time::Instant; +mod bindings; mod keyed_event; mod waitaddress; @@ -60,7 +61,7 @@ impl Backend { Err(global_backend_ptr) => { unsafe { // We lost the race, free our object and return the global one - Box::from_raw(backend_ptr); + let _ = Box::from_raw(backend_ptr); &*global_backend_ptr } } @@ -163,26 +164,10 @@ impl UnparkHandle { // Yields the rest of the current timeslice to the OS #[inline] pub fn thread_yield() { - // Note that this is manually defined here rather than using the definition - // through `winapi`. The `winapi` definition comes from the `synchapi` - // header which enables the "synchronization.lib" library. It turns out, - // however that `Sleep` comes from `kernel32.dll` so this activation isn't - // necessary. - // - // This was originally identified in rust-lang/rust where on MinGW the - // libsynchronization.a library pulls in a dependency on a newer DLL not - // present in older versions of Windows. (see rust-lang/rust#49438) - // - // This is a bit of a hack for now and ideally we'd fix MinGW's own import - // libraries, but that'll probably take a lot longer than patching this here - // and avoiding the `synchapi` feature entirely. - extern "system" { - fn Sleep(a: u32); - } unsafe { // We don't use SwitchToThread here because it doesn't consider all // threads in the system and the thread we are waiting for may not get // selected. - Sleep(0); + bindings::Sleep(0); } } diff --git a/core/src/thread_parker/windows/waitaddress.rs b/core/src/thread_parker/windows/waitaddress.rs index ef6cb44e..e4844647 100644 --- a/core/src/thread_parker/windows/waitaddress.rs +++ b/core/src/thread_parker/windows/waitaddress.rs @@ -10,30 +10,17 @@ use core::{ sync::atomic::{AtomicUsize, Ordering}, }; use std::{ffi, time::Instant}; -use windows_sys::Win32::{ - Foundation::{GetLastError, BOOL, ERROR_TIMEOUT}, - System::{ - LibraryLoader::{GetModuleHandleA, GetProcAddress}, - WindowsProgramming::INFINITE, - }, -}; +use super::bindings::*; #[allow(non_snake_case)] pub struct WaitAddress { - WaitOnAddress: extern "system" fn( - Address: *mut ffi::c_void, - CompareAddress: *mut ffi::c_void, - AddressSize: usize, - dwMilliseconds: u32, - ) -> BOOL, - WakeByAddressSingle: extern "system" fn(Address: *mut ffi::c_void), + WaitOnAddress: WaitOnAddress, + WakeByAddressSingle: WakeByAddressSingle, } impl WaitAddress { #[allow(non_snake_case)] pub fn create() -> Option { - // MSDN claims that that WaitOnAddress and WakeByAddressSingle are - // located in kernel32.dll, but they are lying... let synch_dll = unsafe { GetModuleHandleA(b"api-ms-win-core-synch-l1-2-0.dll\0".as_ptr()) }; if synch_dll == 0 { return None; @@ -108,12 +95,14 @@ impl WaitAddress { #[inline] fn wait_on_address(&'static self, key: &AtomicUsize, timeout: u32) -> BOOL { let cmp = 1usize; - (self.WaitOnAddress)( - key as *const _ as *mut ffi::c_void, - &cmp as *const _ as *mut ffi::c_void, - mem::size_of::(), - timeout, - ) + unsafe { + (self.WaitOnAddress)( + key as *const _ as *mut ffi::c_void, + &cmp as *const _ as *mut ffi::c_void, + mem::size_of::(), + timeout, + ) + } } } @@ -130,6 +119,6 @@ impl UnparkHandle { // released to avoid blocking the queue for too long. #[inline] pub fn unpark(self) { - (self.waitaddress.WakeByAddressSingle)(self.key as *mut ffi::c_void); + unsafe { (self.waitaddress.WakeByAddressSingle)(self.key as *mut ffi::c_void) }; } } From 53081fa6d9201a06ec948f0ab799eee8fd5a7074 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 20 Mar 2023 17:01:03 +0100 Subject: [PATCH 2/4] Add comment detailing the reasoning for manual bindings --- core/src/thread_parker/windows/bindings.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/thread_parker/windows/bindings.rs b/core/src/thread_parker/windows/bindings.rs index 996eb4e6..6bfd8a80 100644 --- a/core/src/thread_parker/windows/bindings.rs +++ b/core/src/thread_parker/windows/bindings.rs @@ -1,3 +1,8 @@ +//! Manual bindings to the win32 API to avoid dependencies on windows-sys or winapi +//! as these bindings will **never** change and parking_lot_core is a foundational +//! dependency for the Rust ecosystem, so the dependencies used by it have an +//! outsize affect + pub const INFINITE: u32 = 4294967295; pub const ERROR_TIMEOUT: u32 = 1460; pub const GENERIC_READ: u32 = 2147483648; From 1df5f7e1e4fe376574470e4a6b0c4e69a091a644 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 20 Mar 2023 17:06:01 +0100 Subject: [PATCH 3/4] Remove pointless casting --- core/src/thread_parker/windows/bindings.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/thread_parker/windows/bindings.rs b/core/src/thread_parker/windows/bindings.rs index 6bfd8a80..730aaa86 100644 --- a/core/src/thread_parker/windows/bindings.rs +++ b/core/src/thread_parker/windows/bindings.rs @@ -7,8 +7,8 @@ pub const INFINITE: u32 = 4294967295; pub const ERROR_TIMEOUT: u32 = 1460; pub const GENERIC_READ: u32 = 2147483648; pub const GENERIC_WRITE: u32 = 1073741824; -pub const STATUS_SUCCESS: i32 = 0 as _; -pub const STATUS_TIMEOUT: i32 = 258 as _; +pub const STATUS_SUCCESS: i32 = 0; +pub const STATUS_TIMEOUT: i32 = 258; pub type HANDLE = isize; pub type HINSTANCE = isize; From d6b74d125ed2f4012cb2186e3bf2e5024bede8ff Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 20 Mar 2023 17:07:45 +0100 Subject: [PATCH 4/4] Removing trailing empty line --- core/src/thread_parker/windows/bindings.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/thread_parker/windows/bindings.rs b/core/src/thread_parker/windows/bindings.rs index 730aaa86..5e1205a5 100644 --- a/core/src/thread_parker/windows/bindings.rs +++ b/core/src/thread_parker/windows/bindings.rs @@ -34,4 +34,3 @@ extern "system" { pub fn Sleep(dwMilliseconds: u32); } -