juamedgod requested jameysharp for a review on PR #6348.
juamedgod requested wasmtime-core-reviewers for a review on PR #6348.
juamedgod opened PR #6348 from juamedgod:windows-opendir-fix
to bytecodealliance:main
:
Description
We were testing the new wasmtime 8.0.1 on Windows and are getting "Permission denied" errors opening directories, while reading files under those directories work fine.
Test Case
The issue is reproducible using the below rust program (built as lister.wasm in the snippets) that directly calls opendir:
extern crate libc; use std::ffi::CString; use libc::{opendir, closedir, c_char}; use std::env; fn main() { let args: Vec<String> = env::args().collect(); if args.len() != 2 { println!("Usage: listdir <directory>"); return; } let dir_name_cstring = CString::new(args[1].clone()).unwrap(); unsafe { let dir_ptr = opendir(dir_name_cstring.as_ptr() as *const c_char); if dir_ptr.is_null() { println!("Error opening directory"); return; } else { closedir(dir_ptr); println!("Opening directory worked"); } } }
Steps to Reproduce
Build a wasm module using the test case snippet, and invoke it pointing to a directory using wasmtime-8.0.1 on Windows:
$> c:\tests\wasmtime-v8.0.1-x86_64-windows\wasmtime.exe --mapdir /app::c:/tests/ c:\tests\lister.wasm -- /app Error opening directory
The same command works when using 8.0.0:
$> c:\tests\wasmtime-v8.0.0-x86_64-windows\wasmtime.exe --mapdir /app::c:/tests/ c:\tests\lister.wasm -- /app Opening directory worked
Versions and Environment
Wasmtime version or commit: wasmtime 8.0.1
Operating system: Windows 10
Architecture: x86_64Extra details
The bug seems to be related to the changes in #6163. The code is failing here:
Which was not previously applied to directories. That results on the directory being reopened which ends up calling Windows ReOpenFile, and that returns a "Access denied".
We have "fixed" the issue by moving the code to be executed only for files, but that may not be the best solution. It would be great of you could give us some tips if that is not the proper solution to improve the patch.
pchickey submitted PR review:
Thank you for finding and fixing this bug!
jameysharp submitted PR review:
This fix makes sense, and thank you for the new tests as well!
jameysharp merged PR #6348.
Last updated: Nov 22 2024 at 16:03 UTC