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.

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

fix: windows implement of op::Write is incorrect #294

Merged
merged 2 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion monoio/src/driver/op/open.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,15 @@ impl OpAble for Open {

#[cfg(all(any(feature = "legacy", feature = "poll-io"), windows))]
fn legacy_call(&mut self) -> io::Result<u32> {
use std::{ffi::OsString, os::windows::ffi::OsStrExt};

let os_str = OsString::from(self.path.to_string_lossy().into_owned());

// Convert OsString to wide character format (Vec<u16>).
let wide_path: Vec<u16> = os_str.encode_wide().chain(Some(0)).collect();
syscall!(
CreateFileW(
self.path.as_c_str().as_ptr().cast(),
wide_path.as_ptr(),
self.opts.access_mode()?,
self.opts.share_mode,
self.opts.security_attributes,
Expand Down
6 changes: 4 additions & 2 deletions monoio/src/driver/op/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use {
windows_sys::Win32::{
Foundation::TRUE,
Networking::WinSock::{WSAGetLastError, WSARecv, WSAESHUTDOWN},
Storage::FileSystem::{ReadFile, SetFilePointer, FILE_CURRENT, INVALID_SET_FILE_POINTER},
Storage::FileSystem::{ReadFile, SetFilePointer, FILE_BEGIN, INVALID_SET_FILE_POINTER},
},
};

Expand Down Expand Up @@ -111,7 +111,9 @@ impl<T: IoBufMut> OpAble for Read<T> {
let ret = unsafe {
// see https://learn.microsoft.com/zh-cn/windows/win32/api/fileapi/nf-fileapi-setfilepointer
if seek_offset != 0 {
let r = SetFilePointer(fd, seek_offset, std::ptr::null_mut(), FILE_CURRENT);
// We use `FILE_BEGIN` because this behavior should be the same with unix syscall
// `pwrite`, which uses the offset from the begin of the file.
let r = SetFilePointer(fd, seek_offset, std::ptr::null_mut(), FILE_BEGIN);
if INVALID_SET_FILE_POINTER == r {
return Err(io::Error::last_os_error());
}
Expand Down
6 changes: 4 additions & 2 deletions monoio/src/driver/op/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use io_uring::{opcode, types};
use windows_sys::Win32::{
Foundation::TRUE,
Networking::WinSock::WSASend,
Storage::FileSystem::{SetFilePointer, WriteFile, FILE_CURRENT, INVALID_SET_FILE_POINTER},
Storage::FileSystem::{SetFilePointer, WriteFile, FILE_BEGIN, INVALID_SET_FILE_POINTER},
};

use super::{super::shared_fd::SharedFd, Op, OpAble};
Expand Down Expand Up @@ -95,7 +95,9 @@ impl<T: IoBuf> OpAble for Write<T> {
let ret = unsafe {
// see https://learn.microsoft.com/zh-cn/windows/win32/api/fileapi/nf-fileapi-setfilepointer
if seek_offset != 0 {
let r = SetFilePointer(fd, seek_offset, std::ptr::null_mut(), FILE_CURRENT);
// We use `FILE_BEGIN` because this behavior should be the same with unix syscall
// `pwrite`, which uses the offset from the begin of the file.
let r = SetFilePointer(fd, seek_offset, std::ptr::null_mut(), FILE_BEGIN);
if INVALID_SET_FILE_POINTER == r {
return Err(io::Error::last_os_error());
}
Expand Down
17 changes: 12 additions & 5 deletions monoio/src/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ pub use file_type::FileType;

#[cfg(unix)]
mod permissions;
#[cfg(windows)]
use std::os::windows::io::{AsRawHandle, FromRawHandle, IntoRawHandle};

#[cfg(unix)]
pub use permissions::Permissions;

Expand All @@ -38,16 +41,20 @@ use crate::buf::IoBuf;
use crate::driver::op::Op;

/// Read the entire contents of a file into a bytes vector.
#[cfg(unix)]
pub async fn read<P: AsRef<Path>>(path: P) -> io::Result<Vec<u8>> {
use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd};

use crate::buf::IoBufMut;

let file = File::open(path).await?;
let sys_file = unsafe { std::fs::File::from_raw_fd(file.as_raw_fd()) };

#[cfg(windows)]
let sys_file = unsafe { std::fs::File::from_raw_handle(file.as_raw_handle()) };
#[cfg(windows)]
let size = sys_file.metadata()?.len() as usize;
let _ = sys_file.into_raw_fd();
#[cfg(windows)]
let _ = sys_file.into_raw_handle();

#[cfg(unix)]
let size = file.metadata().await?.len() as usize;

let (res, buf) = file
.read_exact_at(Vec::with_capacity(size).slice_mut(0..size), 0)
Expand Down
13 changes: 8 additions & 5 deletions monoio/src/time/interval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ use crate::{
time::{sleep_until, Duration, Instant, Sleep},
};

/// Creates new [`Interval`] that yields with interval of `period`. The first
/// tick completes immediately. The default [`MissedTickBehavior`] is
/// Creates new [`Interval`] that yields with interval of `period`.
///
/// The first tick completes immediately. The default [`MissedTickBehavior`] is
/// [`Burst`](MissedTickBehavior::Burst), but this can be configured
/// by calling [`set_missed_tick_behavior`](Interval::set_missed_tick_behavior).
///
Expand Down Expand Up @@ -79,9 +80,11 @@ pub fn interval(period: Duration) -> Interval {
}

/// Creates new [`Interval`] that yields with interval of `period` with the
/// first tick completing at `start`. The default [`MissedTickBehavior`] is
/// [`Burst`](MissedTickBehavior::Burst), but this can be configured
/// by calling [`set_missed_tick_behavior`](Interval::set_missed_tick_behavior).
/// first tick completing at `start`.
///
/// The default [`MissedTickBehavior`] is [`Burst`](MissedTickBehavior::Burst),
/// but this can be configured by calling
/// [`set_missed_tick_behavior`](Interval::set_missed_tick_behavior).
///
/// An interval will tick indefinitely. At any time, the [`Interval`] value can
/// be dropped. This cancels the interval.
Expand Down
49 changes: 45 additions & 4 deletions monoio/tests/fs_file.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// todo fix these CI in windows
#![cfg(not(windows))]
use std::io::prelude::*;
use std::io::{prelude::*, SeekFrom};
#[cfg(unix)]
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
#[cfg(windows)]
Expand Down Expand Up @@ -55,7 +53,7 @@ async fn basic_write() {
file.write_at(HELLO, 0).await.0.unwrap();
file.sync_all().await.unwrap();

let file = std::fs::read(tempfile.path()).unwrap();
let file = monoio::fs::read(tempfile.path()).await.unwrap();
assert_eq!(file, HELLO);
}

Expand Down Expand Up @@ -167,6 +165,7 @@ async fn poll_once(future: impl std::future::Future) {
.await;
}

#[allow(unused, clippy::needless_return)]
fn assert_invalid_fd(fd: RawFd, base: std::fs::Metadata) {
use std::fs::File;
#[cfg(unix)]
Expand Down Expand Up @@ -194,6 +193,7 @@ fn assert_invalid_fd(fd: RawFd, base: std::fs::Metadata) {
}
}

#[cfg(unix)]
#[monoio::test_all]
async fn file_from_std() {
let tempfile = tempfile();
Expand All @@ -208,3 +208,44 @@ async fn file_from_std() {
file.sync_all().await.unwrap();
read_hello(&file).await;
}

#[monoio::test_all]
async fn position_read() {
let mut tempfile = tempfile();
tempfile.write_all(HELLO).unwrap();
tempfile.as_file_mut().sync_data().unwrap();

let file = File::open(tempfile.path()).await.unwrap();

// Modify the file pointer.
let mut std_file = std::fs::File::open(tempfile.path()).unwrap();
std_file.seek(SeekFrom::Start(8)).unwrap();

let buf = Vec::with_capacity(1024);
let (res, buf) = file.read_at(buf, 4).await;
let n = res.unwrap();

assert!(n > 0 && n <= HELLO.len() - 4);
assert_eq!(&buf, &HELLO[4..4 + n]);
}

#[monoio::test_all]
async fn position_write() {
let tempfile = tempfile();

let file = File::create(tempfile.path()).await.unwrap();
file.write_at(HELLO, 0).await.0.unwrap();
file.sync_all().await.unwrap();

// Modify the file pointer.
let mut std_file = std::fs::File::open(tempfile.path()).unwrap();
std_file.seek(SeekFrom::Start(8)).unwrap();

file.write_at(b"monoio...", 6).await.0.unwrap();
file.sync_all().await.unwrap();

assert_eq!(
monoio::fs::read(tempfile.path()).await.unwrap(),
b"hello monoio..."
)
}
19 changes: 19 additions & 0 deletions monoio/tests/fs_read.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use tempfile::NamedTempFile;

const HELLO: &[u8] = b"hello world...";

fn tempfile() -> NamedTempFile {
NamedTempFile::new().expect("unable to create tempfile")
}

#[monoio::test_all]
async fn read_file_all() {
use std::io::Write;

let mut tempfile = tempfile();
tempfile.write_all(HELLO).unwrap();
tempfile.as_file_mut().sync_data().unwrap();

let res = monoio::fs::read(tempfile.path()).await.unwrap();
assert_eq!(res, HELLO);
}
Loading