Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use official windows-sys crate for WinAPI instead of deprecated winapi crate #457

Merged
merged 1 commit into from
Dec 5, 2022
Merged

Use official windows-sys crate for WinAPI instead of deprecated winapi crate #457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion notify/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ kqueue = { version = "1.0", optional = true }
mio = { version = "0.8", features = ["os-ext"], optional = true }

[target.'cfg(windows)'.dependencies]
winapi = { version = "0.3.8", features = ["fileapi", "handleapi", "ioapiset", "minwinbase", "synchapi", "winbase", "winnt"] }
windows-sys = { version = "0.42.0", features = ["Win32_System_Threading", "Win32_Foundation", "Win32_Storage_FileSystem", "Win32_Security", "Win32_System_WindowsProgramming", "Win32_System_IO"] }

[target.'cfg(any(target_os="freebsd", target_os="openbsd", target_os = "netbsd", target_os = "dragonflybsd"))'.dependencies]
kqueue = "^1.0.4" # fix for #344
Expand Down
102 changes: 53 additions & 49 deletions notify/src/windows.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,7 @@
//!
//! [ref]: https://msdn.microsoft.com/en-us/library/windows/desktop/aa363950(v=vs.85).aspx

use winapi::shared::minwindef::TRUE;
use winapi::shared::winerror::ERROR_OPERATION_ABORTED;
use winapi::um::fileapi;
use winapi::um::handleapi::{self, INVALID_HANDLE_VALUE};
use winapi::um::ioapiset;
use winapi::um::minwinbase::{LPOVERLAPPED, OVERLAPPED};
use winapi::um::synchapi;
use winapi::um::winbase::{self, INFINITE, WAIT_OBJECT_0};
use winapi::um::winnt::{self, FILE_NOTIFY_INFORMATION, HANDLE};

use crate::{bounded, unbounded, BoundSender, Receiver, Sender, Config};
use crate::{bounded, unbounded, BoundSender, Config, Receiver, Sender};
use crate::{event::*, WatcherKind};
use crate::{Error, EventHandler, RecursiveMode, Result, Watcher};
use std::collections::HashMap;
Expand All @@ -29,6 +19,23 @@ use std::ptr;
use std::slice;
use std::sync::{Arc, Mutex};
use std::thread;
use windows_sys::Win32::Foundation::{
CloseHandle, ERROR_OPERATION_ABORTED, HANDLE, INVALID_HANDLE_VALUE, WAIT_OBJECT_0,
};
use windows_sys::Win32::Storage::FileSystem::{
CreateFileW, ReadDirectoryChangesW, FILE_ACTION_ADDED, FILE_ACTION_MODIFIED,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to just do ::* for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

windows_sys::Win32::Storage::FileSystem is pretty big, so ::* may lead to great scope pollution…
I am thinking about use windows_sys::Win32::Storage::FileSystem as fileapi, so it will have a short name, like in winapi.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sometimes use a custom prelude module for that

mod win  {
    pub use windows_sys::Win32::Foundation::*;
    pub use windows_sys::Win32::Storage::*;
    pub use windows_sys::Win32::Threading::*;
    pub use windows_sys::Win32::WindowsProgramming::*;
    pub use windows_sys::Win32::IO::*;
}

// ...
handle = win::CreateFileW(
    encoded_path.as_ptr(),
    win::FILE_LIST_DIRECTORY,
    win::FILE_SHARE_READ | win::FILE_SHARE_DELETE | win::FILE_SHARE_WRITE,
    ptr::null_mut(),
    win::OPEN_EXISTING,
    win::FILE_FLAG_BACKUP_SEMANTICS | win::FILE_FLAG_OVERLAPPED,
    0,
);

FILE_ACTION_REMOVED, FILE_ACTION_RENAMED_NEW_NAME, FILE_ACTION_RENAMED_OLD_NAME,
FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OVERLAPPED, FILE_LIST_DIRECTORY,
FILE_NOTIFY_CHANGE_ATTRIBUTES, FILE_NOTIFY_CHANGE_CREATION, FILE_NOTIFY_CHANGE_DIR_NAME,
FILE_NOTIFY_CHANGE_FILE_NAME, FILE_NOTIFY_CHANGE_LAST_WRITE, FILE_NOTIFY_CHANGE_SECURITY,
FILE_NOTIFY_CHANGE_SIZE, FILE_NOTIFY_INFORMATION, FILE_SHARE_DELETE, FILE_SHARE_READ,
FILE_SHARE_WRITE, OPEN_EXISTING,
};
use windows_sys::Win32::System::Threading::{
CreateSemaphoreW, ReleaseSemaphore, WaitForSingleObjectEx,
};
use windows_sys::Win32::System::WindowsProgramming::INFINITE;
use windows_sys::Win32::System::IO::{CancelIo, OVERLAPPED};

const BUF_SIZE: u32 = 16384;

Expand Down Expand Up @@ -132,7 +139,7 @@ impl ReadDirectoryChangesServer {

unsafe {
// wait with alertable flag so that the completion routine fires
let waitres = synchapi::WaitForSingleObjectEx(self.wakeup_sem, 100, TRUE);
let waitres = WaitForSingleObjectEx(self.wakeup_sem, 100, 1);
if waitres == WAIT_OBJECT_0 {
let _ = self.meta_tx.send(MetaEvent::WatcherAwakened);
}
Expand All @@ -141,7 +148,7 @@ impl ReadDirectoryChangesServer {

// we have to clean this up, since the watcher may be long gone
unsafe {
handleapi::CloseHandle(self.wakeup_sem);
CloseHandle(self.wakeup_sem);
}
}

Expand Down Expand Up @@ -170,14 +177,14 @@ impl ReadDirectoryChangesServer {
.collect();
let handle;
unsafe {
handle = fileapi::CreateFileW(
handle = CreateFileW(
encoded_path.as_ptr(),
winnt::FILE_LIST_DIRECTORY,
winnt::FILE_SHARE_READ | winnt::FILE_SHARE_DELETE | winnt::FILE_SHARE_WRITE,
ptr::null_mut(),
fileapi::OPEN_EXISTING,
winbase::FILE_FLAG_BACKUP_SEMANTICS | winbase::FILE_FLAG_OVERLAPPED,
FILE_LIST_DIRECTORY,
FILE_SHARE_READ | FILE_SHARE_DELETE | FILE_SHARE_WRITE,
ptr::null_mut(),
OPEN_EXISTING,
FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OVERLAPPED,
0,
);

if handle == INVALID_HANDLE_VALUE {
Expand All @@ -199,11 +206,10 @@ impl ReadDirectoryChangesServer {
None
};
// every watcher gets its own semaphore to signal completion
let semaphore =
unsafe { synchapi::CreateSemaphoreW(ptr::null_mut(), 0, 1, ptr::null_mut()) };
if semaphore.is_null() || semaphore == INVALID_HANDLE_VALUE {
let semaphore = unsafe { CreateSemaphoreW(ptr::null_mut(), 0, 1, ptr::null_mut()) };
if semaphore == 0 || semaphore == INVALID_HANDLE_VALUE {
unsafe {
handleapi::CloseHandle(handle);
CloseHandle(handle);
}
return Err(Error::generic("Failed to create semaphore for watch.").add_path(path));
}
Expand Down Expand Up @@ -236,16 +242,15 @@ impl ReadDirectoryChangesServer {

fn stop_watch(ws: &WatchState, meta_tx: &Sender<MetaEvent>) {
unsafe {
let cio = ioapiset::CancelIo(ws.dir_handle);
let ch = handleapi::CloseHandle(ws.dir_handle);
let cio = CancelIo(ws.dir_handle);
let ch = CloseHandle(ws.dir_handle);
// have to wait for it, otherwise we leak the memory allocated for there read request
if cio != 0 && ch != 0 {
while synchapi::WaitForSingleObjectEx(ws.complete_sem, INFINITE, TRUE) != WAIT_OBJECT_0
{
while WaitForSingleObjectEx(ws.complete_sem, INFINITE, 1) != WAIT_OBJECT_0 {
// drain the apc queue, fix for https://github.com/notify-rs/notify/issues/287#issuecomment-801465550
}
}
handleapi::CloseHandle(ws.complete_sem);
CloseHandle(ws.complete_sem);
}
let _ = meta_tx.send(MetaEvent::SingleWatchComplete);
}
Expand All @@ -258,13 +263,13 @@ fn start_read(rd: &ReadData, event_handler: Arc<Mutex<dyn EventHandler>>, handle
data: rd.clone(),
});

let flags = winnt::FILE_NOTIFY_CHANGE_FILE_NAME
| winnt::FILE_NOTIFY_CHANGE_DIR_NAME
| winnt::FILE_NOTIFY_CHANGE_ATTRIBUTES
| winnt::FILE_NOTIFY_CHANGE_SIZE
| winnt::FILE_NOTIFY_CHANGE_LAST_WRITE
| winnt::FILE_NOTIFY_CHANGE_CREATION
| winnt::FILE_NOTIFY_CHANGE_SECURITY;
let flags = FILE_NOTIFY_CHANGE_FILE_NAME
| FILE_NOTIFY_CHANGE_DIR_NAME
| FILE_NOTIFY_CHANGE_ATTRIBUTES
| FILE_NOTIFY_CHANGE_SIZE
| FILE_NOTIFY_CHANGE_LAST_WRITE
| FILE_NOTIFY_CHANGE_CREATION
| FILE_NOTIFY_CHANGE_SECURITY;

let monitor_subdir = if (&request.data.file).is_none() && request.data.is_recursive {
1
Expand All @@ -278,12 +283,12 @@ fn start_read(rd: &ReadData, event_handler: Arc<Mutex<dyn EventHandler>>, handle
// for our own purposes

let req_buf = request.buffer.as_mut_ptr() as *mut c_void;
let request_p = Box::into_raw(request) as *mut c_void;
let request_p = Box::into_raw(request) as isize;
overlapped.hEvent = request_p;

// This is using an asynchronous call with a completion routine for receiving notifications
// An I/O completion port would probably be more performant
let ret = winbase::ReadDirectoryChangesW(
let ret = ReadDirectoryChangesW(
handle,
req_buf,
BUF_SIZE,
Expand All @@ -299,7 +304,7 @@ fn start_read(rd: &ReadData, event_handler: Arc<Mutex<dyn EventHandler>>, handle
// allow overlapped to drop by omitting forget()
let request: Box<ReadDirectoryRequest> = mem::transmute(request_p);

synchapi::ReleaseSemaphore(request.data.complete_sem, 1, ptr::null_mut());
ReleaseSemaphore(request.data.complete_sem, 1, ptr::null_mut());
} else {
// read ok. forget overlapped to let the completion routine handle memory
mem::forget(overlapped);
Expand All @@ -310,15 +315,15 @@ fn start_read(rd: &ReadData, event_handler: Arc<Mutex<dyn EventHandler>>, handle
unsafe extern "system" fn handle_event(
error_code: u32,
_bytes_written: u32,
overlapped: LPOVERLAPPED,
overlapped: *mut OVERLAPPED,
) {
let overlapped: Box<OVERLAPPED> = Box::from_raw(overlapped);
let request: Box<ReadDirectoryRequest> = Box::from_raw(overlapped.hEvent as *mut _);

if error_code == ERROR_OPERATION_ABORTED {
// received when dir is unwatched or watcher is shutdown; return and let overlapped/request
// get drop-cleaned
synchapi::ReleaseSemaphore(request.data.complete_sem, 1, ptr::null_mut());
ReleaseSemaphore(request.data.complete_sem, 1, ptr::null_mut());
return;
}

Expand Down Expand Up @@ -359,30 +364,30 @@ unsafe extern "system" fn handle_event(

let event_handler = |res| emit_event(&request.event_handler, res);

if (*cur_entry).Action == winnt::FILE_ACTION_RENAMED_OLD_NAME {
if (*cur_entry).Action == FILE_ACTION_RENAMED_OLD_NAME {
let mode = RenameMode::From;
let kind = ModifyKind::Name(mode);
let kind = EventKind::Modify(kind);
let ev = newe.set_kind(kind);
event_handler(Ok(ev))
} else {
match (*cur_entry).Action {
winnt::FILE_ACTION_RENAMED_NEW_NAME => {
FILE_ACTION_RENAMED_NEW_NAME => {
let kind = EventKind::Modify(ModifyKind::Name(RenameMode::To));
let ev = newe.set_kind(kind);
event_handler(Ok(ev));
}
winnt::FILE_ACTION_ADDED => {
FILE_ACTION_ADDED => {
let kind = EventKind::Create(CreateKind::Any);
let ev = newe.set_kind(kind);
event_handler(Ok(ev));
}
winnt::FILE_ACTION_REMOVED => {
FILE_ACTION_REMOVED => {
let kind = EventKind::Remove(RemoveKind::Any);
let ev = newe.set_kind(kind);
event_handler(Ok(ev));
}
winnt::FILE_ACTION_MODIFIED => {
FILE_ACTION_MODIFIED => {
let kind = EventKind::Modify(ModifyKind::Any);
let ev = newe.set_kind(kind);
event_handler(Ok(ev));
Expand Down Expand Up @@ -415,9 +420,8 @@ impl ReadDirectoryChangesWatcher {
) -> Result<ReadDirectoryChangesWatcher> {
let (cmd_tx, cmd_rx) = unbounded();

let wakeup_sem =
unsafe { synchapi::CreateSemaphoreW(ptr::null_mut(), 0, 1, ptr::null_mut()) };
if wakeup_sem.is_null() || wakeup_sem == INVALID_HANDLE_VALUE {
let wakeup_sem = unsafe { CreateSemaphoreW(ptr::null_mut(), 0, 1, ptr::null_mut()) };
if wakeup_sem == 0 || wakeup_sem == INVALID_HANDLE_VALUE {
0xpr03 marked this conversation as resolved.
Show resolved Hide resolved
return Err(Error::generic("Failed to create wakeup semaphore."));
}

Expand All @@ -436,7 +440,7 @@ impl ReadDirectoryChangesWatcher {
// so that if you add a watch you don't block for 100ms in watch() while the
// server sleeps.
unsafe {
synchapi::ReleaseSemaphore(self.wakeup_sem, 1, ptr::null_mut());
ReleaseSemaphore(self.wakeup_sem, 1, ptr::null_mut());
}
}

Expand Down