mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
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 (#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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
ce5961b1ec
commit
13e000dc20
@@ -44,15 +44,42 @@ jobs:
|
||||
- name: Check lints
|
||||
run: cargo clippy --all --benches --tests --examples ${{ matrix.flags }} -- -D warnings
|
||||
|
||||
clippy-windows:
|
||||
name: Clippy Windows (${{ matrix.name }})
|
||||
runs-on: windows-latest
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
include:
|
||||
- name: all-features
|
||||
flags: "--all-features"
|
||||
- name: default
|
||||
flags: ""
|
||||
- name: libsql-only
|
||||
flags: "--no-default-features --features libsql"
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v6
|
||||
- name: Install Rust
|
||||
uses: dtolnay/rust-toolchain@stable
|
||||
with:
|
||||
profile: minimal
|
||||
components: clippy
|
||||
- uses: Swatinem/rust-cache@v2
|
||||
with:
|
||||
key: clippy-windows-${{ matrix.name }}
|
||||
- name: Check lints
|
||||
run: cargo clippy --all --benches --tests --examples ${{ matrix.flags }} -- -D warnings
|
||||
|
||||
# Roll-up job for branch protection
|
||||
code-style:
|
||||
name: Code Style (fmt + clippy)
|
||||
runs-on: ubuntu-latest
|
||||
if: always()
|
||||
needs: [format, clippy]
|
||||
needs: [format, clippy, clippy-windows]
|
||||
steps:
|
||||
- run: |
|
||||
if [[ "${{ needs.format.result }}" != "success" || "${{ needs.clippy.result }}" != "success" ]]; then
|
||||
if [[ "${{ needs.format.result }}" != "success" || "${{ needs.clippy.result }}" != "success" || "${{ needs.clippy-windows.result }}" != "success" ]]; then
|
||||
echo "One or more jobs failed"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -51,6 +51,32 @@ jobs:
|
||||
- name: Run Telegram Channel Tests
|
||||
run: cargo test --manifest-path channels-src/telegram/Cargo.toml -- --nocapture
|
||||
|
||||
windows-build:
|
||||
name: Windows Build (${{ matrix.name }})
|
||||
runs-on: windows-latest
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
include:
|
||||
- name: all-features
|
||||
flags: "--all-features"
|
||||
- name: default
|
||||
flags: ""
|
||||
- name: libsql-only
|
||||
flags: "--no-default-features --features libsql"
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v6
|
||||
- name: Install Rust
|
||||
uses: dtolnay/rust-toolchain@stable
|
||||
with:
|
||||
profile: minimal
|
||||
- uses: Swatinem/rust-cache@v2
|
||||
with:
|
||||
key: windows-${{ matrix.name }}
|
||||
- name: Check compilation
|
||||
run: cargo check --all --benches --tests --examples ${{ matrix.flags }}
|
||||
|
||||
wasm-wit-compat:
|
||||
name: WASM WIT Compatibility
|
||||
runs-on: ubuntu-latest
|
||||
@@ -100,10 +126,10 @@ jobs:
|
||||
name: Run Tests
|
||||
runs-on: ubuntu-latest
|
||||
if: always()
|
||||
needs: [tests, telegram-tests, wasm-wit-compat, docker-build, version-check]
|
||||
needs: [tests, telegram-tests, wasm-wit-compat, docker-build, windows-build, version-check]
|
||||
steps:
|
||||
- run: |
|
||||
if [[ "${{ needs.tests.result }}" != "success" || "${{ needs.telegram-tests.result }}" != "success" || "${{ needs.wasm-wit-compat.result }}" != "success" || "${{ needs.docker-build.result }}" != "success" ]]; then
|
||||
if [[ "${{ needs.tests.result }}" != "success" || "${{ needs.telegram-tests.result }}" != "success" || "${{ needs.wasm-wit-compat.result }}" != "success" || "${{ needs.docker-build.result }}" != "success" || "${{ needs.windows-build.result }}" != "success" ]]; then
|
||||
echo "One or more jobs failed"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -153,7 +153,16 @@ impl WasmChannelRuntime {
|
||||
// Enable persistent compilation cache. Wasmtime serializes compiled native
|
||||
// code to disk (~/.cache/wasmtime by default), so subsequent startups
|
||||
// deserialize instead of recompiling — typically 10-50x faster.
|
||||
if let Err(e) = wasmtime_config.cache_config_load_default() {
|
||||
//
|
||||
// On Windows, each Engine gets its own cache subdirectory to avoid
|
||||
// OS error 33 (ERROR_LOCK_VIOLATION) when multiple engines share the
|
||||
// default cache and Windows holds exclusive locks on memory-mapped
|
||||
// files. See #448.
|
||||
if let Err(e) = crate::tools::wasm::enable_compilation_cache(
|
||||
&mut wasmtime_config,
|
||||
"channels",
|
||||
config.cache_dir.as_deref(),
|
||||
) {
|
||||
tracing::warn!("Failed to enable wasmtime compilation cache: {}", e);
|
||||
}
|
||||
|
||||
|
||||
@@ -26,7 +26,9 @@
|
||||
//! ```
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::path::Path;
|
||||
#[cfg(unix)]
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
use bollard::Docker;
|
||||
|
||||
@@ -20,9 +20,11 @@
|
||||
use crate::secrets::SecretError;
|
||||
|
||||
/// Service name for keychain entries.
|
||||
#[cfg(any(target_os = "macos", target_os = "linux"))]
|
||||
const SERVICE_NAME: &str = "ironclaw";
|
||||
|
||||
/// Account name for the master key.
|
||||
#[cfg(any(target_os = "macos", target_os = "linux"))]
|
||||
const MASTER_KEY_ACCOUNT: &str = "master_key";
|
||||
|
||||
/// Generate a random 32-byte master key.
|
||||
@@ -261,6 +263,7 @@ mod platform {
|
||||
pub use platform::{delete_master_key, get_master_key, has_master_key, store_master_key};
|
||||
|
||||
/// Parse a hex string to bytes.
|
||||
#[cfg(any(target_os = "macos", target_os = "linux", test))]
|
||||
fn hex_to_bytes(hex: &str) -> Result<Vec<u8>, SecretError> {
|
||||
if !hex.len().is_multiple_of(2) {
|
||||
return Err(SecretError::KeychainError(
|
||||
|
||||
@@ -309,6 +309,7 @@ async fn setup_tunnel_cloudflare() -> Result<TunnelSettings, ChannelSetupError>
|
||||
/// Detect running cloudflared processes or managed services that could conflict
|
||||
/// with IronClaw's tunnel management.
|
||||
fn detect_existing_cloudflared() -> Option<String> {
|
||||
#[allow(unused_mut)]
|
||||
let mut conflicts: Vec<String> = Vec::new();
|
||||
|
||||
// Check for running cloudflared processes (all platforms)
|
||||
|
||||
@@ -102,7 +102,7 @@ pub use limits::{
|
||||
DEFAULT_FUEL_LIMIT, DEFAULT_MEMORY_LIMIT, DEFAULT_TIMEOUT, FuelConfig, ResourceLimits,
|
||||
WasmResourceLimiter,
|
||||
};
|
||||
pub use runtime::{PreparedModule, WasmRuntimeConfig, WasmToolRuntime};
|
||||
pub use runtime::{PreparedModule, WasmRuntimeConfig, WasmToolRuntime, enable_compilation_cache};
|
||||
pub use wrapper::{OAuthRefreshConfig, WasmToolWrapper};
|
||||
|
||||
// Capabilities (V2)
|
||||
|
||||
+118
-2
@@ -4,7 +4,7 @@
|
||||
//! This matches NEAR blockchain patterns for deterministic, isolated execution.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
@@ -18,6 +18,58 @@ use crate::tools::wasm::limits::{FuelConfig, ResourceLimits};
|
||||
/// which causes any store with an expired epoch deadline to trap.
|
||||
pub const EPOCH_TICK_INTERVAL: Duration = Duration::from_millis(500);
|
||||
|
||||
/// Enable wasmtime's persistent compilation cache for a [`Config`].
|
||||
///
|
||||
/// On Unix, this delegates to `cache_config_load_default()` which uses a
|
||||
/// shared cache directory. On Windows, each engine gets its own subdirectory
|
||||
/// (keyed by `label`) to avoid OS error 33 (`ERROR_LOCK_VIOLATION`) when
|
||||
/// multiple engines memory-map files in the same cache directory. See #448.
|
||||
///
|
||||
/// If `explicit_dir` is `Some`, it is used as the cache directory on all
|
||||
/// platforms, bypassing the default.
|
||||
pub fn enable_compilation_cache(
|
||||
wasmtime_config: &mut Config,
|
||||
label: &str,
|
||||
explicit_dir: Option<&Path>,
|
||||
) -> anyhow::Result<()> {
|
||||
// If the caller provided an explicit directory, or we're on Windows and
|
||||
// need per-engine isolation, write a TOML config with a custom directory.
|
||||
let custom_dir = match explicit_dir {
|
||||
Some(dir) => Some(dir.to_path_buf()),
|
||||
#[cfg(windows)]
|
||||
None => {
|
||||
let base = dirs::cache_dir()
|
||||
.unwrap_or_else(std::env::temp_dir)
|
||||
.join("ironclaw");
|
||||
Some(base.join(format!("wasmtime-{}", label)))
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
None => {
|
||||
let _ = label;
|
||||
None
|
||||
}
|
||||
};
|
||||
|
||||
match custom_dir {
|
||||
Some(dir) => {
|
||||
std::fs::create_dir_all(&dir)?;
|
||||
let toml_path = dir.join("wasmtime-cache.toml");
|
||||
let escaped = dir
|
||||
.to_string_lossy()
|
||||
.replace('\\', "\\\\")
|
||||
.replace('"', "\\\"");
|
||||
let toml_content = format!("[cache]\nenabled = true\ndirectory = \"{}\"\n", escaped);
|
||||
std::fs::write(&toml_path, toml_content)?;
|
||||
wasmtime_config.cache_config_load(&toml_path)?;
|
||||
Ok(())
|
||||
}
|
||||
None => {
|
||||
wasmtime_config.cache_config_load_default()?;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Configuration for the WASM runtime.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct WasmRuntimeConfig {
|
||||
@@ -136,7 +188,14 @@ impl WasmToolRuntime {
|
||||
// Enable persistent compilation cache. Wasmtime serializes compiled native
|
||||
// code to disk (~/.cache/wasmtime by default), so subsequent startups
|
||||
// deserialize instead of recompiling — typically 10-50x faster.
|
||||
if let Err(e) = wasmtime_config.cache_config_load_default() {
|
||||
//
|
||||
// On Windows, each Engine gets its own cache subdirectory to avoid
|
||||
// OS error 33 (ERROR_LOCK_VIOLATION) when multiple engines share the
|
||||
// default cache and Windows holds exclusive locks on memory-mapped
|
||||
// files. See #448.
|
||||
if let Err(e) =
|
||||
enable_compilation_cache(&mut wasmtime_config, "tools", config.cache_dir.as_deref())
|
||||
{
|
||||
tracing::warn!("Failed to enable wasmtime compilation cache: {}", e);
|
||||
}
|
||||
|
||||
@@ -348,6 +407,63 @@ mod tests {
|
||||
assert_eq!(limits.fuel, 500_000);
|
||||
}
|
||||
|
||||
/// Per-engine cache directories must work correctly to avoid file lock
|
||||
/// conflicts on Windows where multiple engines sharing a single cache
|
||||
/// directory triggers OS error 33 (ERROR_LOCK_VIOLATION). Regression test
|
||||
/// for #448: `enable_compilation_cache` must create a subdirectory and
|
||||
/// produce a valid TOML config that wasmtime can load.
|
||||
#[test]
|
||||
fn test_enable_compilation_cache_with_explicit_dir() {
|
||||
use crate::tools::wasm::runtime::enable_compilation_cache;
|
||||
|
||||
let tmp = tempfile::tempdir().expect("failed to create temp dir");
|
||||
let cache_dir = tmp.path().join("custom-cache");
|
||||
|
||||
let mut config = wasmtime::Config::new();
|
||||
enable_compilation_cache(&mut config, "test-engine", Some(cache_dir.as_path()))
|
||||
.expect("enable_compilation_cache should succeed with explicit dir");
|
||||
|
||||
// The cache directory should have been created.
|
||||
assert!(cache_dir.exists(), "cache directory should be created");
|
||||
|
||||
// A TOML config file should have been written inside.
|
||||
let toml_path = cache_dir.join("wasmtime-cache.toml");
|
||||
assert!(toml_path.exists(), "TOML config should be written");
|
||||
|
||||
let content = std::fs::read_to_string(&toml_path).unwrap();
|
||||
assert!(
|
||||
content.contains("[cache]"),
|
||||
"TOML must contain [cache] section"
|
||||
);
|
||||
assert!(content.contains("enabled = true"), "cache must be enabled");
|
||||
}
|
||||
|
||||
/// Two engines with different labels must get independent cache directories
|
||||
/// so that their file locks do not conflict. Regression test for #448.
|
||||
#[test]
|
||||
fn test_enable_compilation_cache_label_isolation() {
|
||||
use crate::tools::wasm::runtime::enable_compilation_cache;
|
||||
|
||||
let tmp = tempfile::tempdir().expect("failed to create temp dir");
|
||||
let base = tmp.path().join("isolation");
|
||||
|
||||
let dir_a = base.join("engine-a");
|
||||
let dir_b = base.join("engine-b");
|
||||
|
||||
let mut config_a = wasmtime::Config::new();
|
||||
enable_compilation_cache(&mut config_a, "a", Some(dir_a.as_path()))
|
||||
.expect("cache A should succeed");
|
||||
|
||||
let mut config_b = wasmtime::Config::new();
|
||||
enable_compilation_cache(&mut config_b, "b", Some(dir_b.as_path()))
|
||||
.expect("cache B should succeed");
|
||||
|
||||
// Both directories must exist and be distinct.
|
||||
assert!(dir_a.exists());
|
||||
assert!(dir_b.exists());
|
||||
assert_ne!(dir_a, dir_b);
|
||||
}
|
||||
|
||||
/// The WASM runtime (Wasmtime engine) must initialise successfully even
|
||||
/// when no tools directory exists on disk. The engine only configures the
|
||||
/// compiler and epoch ticker — loading modules from a directory is a
|
||||
|
||||
Reference in New Issue
Block a user