fix(wasm): use per-engine cache dirs on Windows to avoid file lock error#624
fix(wasm): use per-engine cache dirs on Windows to avoid file lock error#624ilblackdragon merged 7 commits intomainfrom
Conversation
…ror (#448) On Windows, multiple wasmtime Engine instances sharing the default compilation cache directory hit OS error 33 (ERROR_LOCK_VIOLATION) because Windows holds exclusive file locks on memory-mapped cache files. This is especially triggered when the Telegram channel WASM module is loaded at startup and then hot-activated via the Extensions UI. Fix by giving each engine its own cache subdirectory on Windows (~/.cache/ironclaw/wasmtime-tools/ and wasmtime-channels/). On Unix the shared default cache continues to work as before. Also adds Windows CI jobs (cargo check + clippy across all feature flag combinations) to catch Windows-specific issues going forward. Closes #448 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Gate PathBuf import behind #[cfg(unix)] in container.rs (only used in Unix socket path), suppress unused_mut on conflicts Vec in channels.rs (mutations are platform-gated), and add cfg gates on keychain constants and hex_to_bytes that are only used on macOS/Linux. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use double-quoted TOML strings with backslash and double-quote escaping for the cache directory path, preventing breakage or injection when paths contain special characters (e.g. single quotes on Unix, backslashes on Windows). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix import ordering in container.rs and line wrapping in runtime.rs to pass the CI formatting check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n-issue-448.-do-we-have
…merge main - Gate `use std::path::PathBuf` with `#[cfg(unix)]` in sandbox/container.rs since all usages are inside `#[cfg(unix)]` functions, fixing Windows clippy error (unused import) - Add two regression tests for `enable_compilation_cache` (#448): one verifies explicit cache directory creation and TOML config, another verifies label-based isolation between engines - Resolve merge conflict in test.yml to include both windows-build and wasm-wit-compat jobs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Path is used in non-cfg-gated functions (lines 148, 244) so it must be available on all platforms. Only PathBuf is unix-specific. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical file locking issue on Windows that occurred when multiple Wasmtime engines attempted to use the same compilation cache directory. By isolating these caches per engine on Windows, the change prevents runtime errors and improves the stability of the application. It also enhances the development process by introducing dedicated CI checks for Windows, ensuring better cross-platform compatibility moving forward. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses a file lock violation on Windows by introducing per-engine cache directories for wasmtime. The implementation correctly isolates cache directories for 'tools' and 'channels' engines on Windows while maintaining the existing shared cache behavior on other platforms. The changes are well-structured, moving the cache configuration logic into a shared function enable_compilation_cache. The addition of new tests for this function is also a great improvement.
My review includes one suggestion to improve the robustness of how the wasmtime cache configuration file is generated, by using the toml crate for serialization instead of manual string manipulation. Overall, this is a solid fix for the reported issue.
| let escaped = dir | ||
| .to_string_lossy() | ||
| .replace('\\', "\\\\") | ||
| .replace('"', "\\\""); | ||
| let toml_content = format!("[cache]\nenabled = true\ndirectory = \"{}\"\n", escaped); |
There was a problem hiding this comment.
Manually escaping characters for TOML generation is brittle and can be error-prone if paths contain unusual characters. It's more robust to use the toml crate to handle serialization and escaping of values. This ensures that the generated TOML is always valid.
You can achieve this by using toml::Value::String, which will correctly quote and escape the path string. This may require adding the toml crate as a dependency if it's not already available.
| let escaped = dir | |
| .to_string_lossy() | |
| .replace('\\', "\\\\") | |
| .replace('"', "\\\""); | |
| let toml_content = format!("[cache]\nenabled = true\ndirectory = \"{}\"\n", escaped); | |
| let toml_content = format!( | |
| "[cache]\nenabled = true\ndirectory = {}\n", | |
| toml::Value::String(dir.to_string_lossy().into_owned()) | |
| ); |
…ror (nearai#624) * fix(wasm): use per-engine cache dirs on Windows to avoid file lock error (nearai#448) On Windows, multiple wasmtime Engine instances sharing the default compilation cache directory hit OS error 33 (ERROR_LOCK_VIOLATION) because Windows holds exclusive file locks on memory-mapped cache files. This is especially triggered when the Telegram channel WASM module is loaded at startup and then hot-activated via the Extensions UI. Fix by giving each engine its own cache subdirectory on Windows (~/.cache/ironclaw/wasmtime-tools/ and wasmtime-channels/). On Unix the shared default cache continues to work as before. Also adds Windows CI jobs (cargo check + clippy across all feature flag combinations) to catch Windows-specific issues going forward. Closes nearai#448 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: silence Windows clippy warnings for platform-gated code Gate PathBuf import behind #[cfg(unix)] in container.rs (only used in Unix socket path), suppress unused_mut on conflicts Vec in channels.rs (mutations are platform-gated), and add cfg gates on keychain constants and hex_to_bytes that are only used on macOS/Linux. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: escape directory path in TOML cache config to prevent injection Use double-quoted TOML strings with backslash and double-quote escaping for the cache directory path, preventing breakage or injection when paths contain special characters (e.g. single quotes on Unix, backslashes on Windows). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve cargo fmt formatting errors Fix import ordering in container.rs and line wrapping in runtime.rs to pass the CI formatting check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): restore Path import for all platforms, keep PathBuf unix-only Path is used in non-cfg-gated functions (lines 148, 244) so it must be available on all platforms. Only PathBuf is unix-specific. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…ror (nearai#624) * fix(wasm): use per-engine cache dirs on Windows to avoid file lock error (nearai#448) On Windows, multiple wasmtime Engine instances sharing the default compilation cache directory hit OS error 33 (ERROR_LOCK_VIOLATION) because Windows holds exclusive file locks on memory-mapped cache files. This is especially triggered when the Telegram channel WASM module is loaded at startup and then hot-activated via the Extensions UI. Fix by giving each engine its own cache subdirectory on Windows (~/.cache/ironclaw/wasmtime-tools/ and wasmtime-channels/). On Unix the shared default cache continues to work as before. Also adds Windows CI jobs (cargo check + clippy across all feature flag combinations) to catch Windows-specific issues going forward. Closes nearai#448 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: silence Windows clippy warnings for platform-gated code Gate PathBuf import behind #[cfg(unix)] in container.rs (only used in Unix socket path), suppress unused_mut on conflicts Vec in channels.rs (mutations are platform-gated), and add cfg gates on keychain constants and hex_to_bytes that are only used on macOS/Linux. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: escape directory path in TOML cache config to prevent injection Use double-quoted TOML strings with backslash and double-quote escaping for the cache directory path, preventing breakage or injection when paths contain special characters (e.g. single quotes on Unix, backslashes on Windows). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve cargo fmt formatting errors Fix import ordering in container.rs and line wrapping in runtime.rs to pass the CI formatting check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): restore Path import for all platforms, keep PathBuf unix-only Path is used in non-cfg-gated functions (lines 148, 244) so it must be available on all platforms. Only PathBuf is unix-specific. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
ERROR_LOCK_VIOLATION) when multiple wasmtime engines share the default compilation cache directory. Each engine now gets its own cache subdirectory on Windows (wasmtime-tools/,wasmtime-channels/), avoiding file lock conflicts on memory-mapped cache files. Unix behavior is unchanged.cargo check+cargo clippyacross all 3 feature flag combinations) to catch Windows-specific build issues going forward.Details
The root cause: IronClaw creates two separate wasmtime
Engineinstances (one for tool WASM modules, one for channel WASM modules). Both calledcache_config_load_default(), which uses a shared cache directory (~/.cache/wasmtime). On Windows, memory-mapped files get exclusive OS-level locks, so the second engine'sComponent::new()fails when it hits the lock from the first.The Telegram channel is especially susceptible because it's typically the first channel set up after onboarding, creating a startup-load + immediate-hot-activation double-load scenario.
The fix introduces
enable_compilation_cache()which:cache_config_load_default()(shared cache, no lock issues)~/.cache/ironclaw/wasmtime-{tools,channels}/Both configs already had an unused
cache_dir: Option<PathBuf>field which is now wired through as an explicit override on all platforms.Test plan
cargo clippy --all --all-features-- zero warningscargo test --lib-- 1845 passed, 0 failedCloses #448
Generated with Claude Code