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

* fix: fix windows position read/write bug

Signed-off-by: Lzzzt <liuzitao0123@gmail.com>

* ci: make clippy happy

Signed-off-by: lzzzt <liuzitao0123@gmail.com>

---------

Signed-off-by: Lzzzt <liuzitao0123@gmail.com>
Signed-off-by: lzzzt <liuzitao0123@gmail.com>

monoio/src/driver/op/open.rs

@@ -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,

monoio/src/driver/op/read.rs

@@ -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},
  },
 };
 
@@ -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());
  }

monoio/src/driver/op/write.rs

@@ -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};
@@ -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());
  }

monoio/src/fs/mod.rs

@@ -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;
 
@@ -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)

monoio/src/time/interval.rs

@@ -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).
 ///
@@ -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.

monoio/tests/fs_file.rs

@@ -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)]
@@ -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);
 }
 
@@ -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)]
@@ -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();
@@ -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..."
+ )
+}

monoio/tests/fs_read.rs

@@ -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);
+}