diff --git a/dotnet/src/Devolutions.Pinget.Cli/Program.cs b/dotnet/src/Devolutions.Pinget.Cli/Program.cs index ddb5dc2..955aac8 100644 --- a/dotnet/src/Devolutions.Pinget.Cli/Program.cs +++ b/dotnet/src/Devolutions.Pinget.Cli/Program.cs @@ -6,7 +6,7 @@ using Devolutions.Pinget.Cli; using Devolutions.Pinget.Core; -const string Version = "0.8.0"; +const string Version = "0.8.1"; const string UpgradeUnsupportedWarning = "Upgrading packages is not supported on this platform; no changes were made."; if (args.Length == 1 && (string.Equals(args[0], "--version", StringComparison.OrdinalIgnoreCase) || string.Equals(args[0], "-v", StringComparison.OrdinalIgnoreCase))) diff --git a/dotnet/src/Devolutions.Pinget.Core/PreIndexedSource.cs b/dotnet/src/Devolutions.Pinget.Core/PreIndexedSource.cs index 6f5fc3b..f9bfce1 100644 --- a/dotnet/src/Devolutions.Pinget.Core/PreIndexedSource.cs +++ b/dotnet/src/Devolutions.Pinget.Core/PreIndexedSource.cs @@ -1,4 +1,5 @@ using System.IO.Compression; +using System.Threading; using Microsoft.Data.Sqlite; using YamlDotNet.Core; using YamlDotNet.Core.Events; @@ -46,7 +47,12 @@ public static string Update(HttpClient client, SourceRecord source, string? appR try { indexEntry.ExtractToFile(tempIndexPath, overwrite: true); - File.Move(tempIndexPath, indexPath, overwrite: true); + ReplaceIndexFile(tempIndexPath, indexPath); + // The new index.db is a fresh database; any WAL/SHM sidecars + // left from the previous one no longer match it and would make + // SQLite recover against the wrong database. Drop them so the + // next open starts clean. + RemoveSqliteSidecars(indexPath); } finally { @@ -67,6 +73,76 @@ public static string Update(HttpClient client, SourceRecord source, string? appR $"Failed to update preindexed source '{source.Name}': {lastError?.Message}"); } + private const int ReplaceRetryAttempts = 5; + + /// + /// Replaces with , surviving the + /// case where another handle holds the target open. A plain overwriting move uses + /// MOVEFILE_REPLACE_EXISTING, which is denied while an on-access AV scan or a + /// concurrent reader has the index open; we retry briefly and then stage the existing + /// file aside (which succeeds because SQLite opens with FILE_SHARE_DELETE), restoring + /// it if the swap fails so a working index is never lost. + /// + private static void ReplaceIndexFile(string source, string target) + { + try + { + File.Move(source, target, overwrite: true); + return; + } + catch (Exception ex) when ((ex is IOException or UnauthorizedAccessException) + && File.Exists(source) && File.Exists(target)) + { + // Fall through to the retry / stage-aside path. + } + + for (var attempt = 1; attempt <= ReplaceRetryAttempts; attempt++) + { + Thread.Sleep(50 * attempt); + try + { + File.Move(source, target, overwrite: true); + return; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // Keep retrying until the holder releases the file. + } + } + + var staged = Path.ChangeExtension(target, "old.tmp"); + File.Move(target, staged, overwrite: true); + try + { + File.Move(source, target, overwrite: true); + TryDelete(staged); + } + catch + { + File.Move(staged, target, overwrite: true); + throw; + } + } + + private static void RemoveSqliteSidecars(string dbPath) + { + TryDelete(dbPath + "-wal"); + TryDelete(dbPath + "-shm"); + } + + private static void TryDelete(string path) + { + try + { + if (File.Exists(path)) + File.Delete(path); + } + catch + { + // Best-effort cleanup; a leftover sidecar is harmless on the next open. + } + } + // V2 search: flat `packages` table with id, name, moniker, latest_version, hash public static (List Rows, bool Truncated) SearchV2( SqliteConnection conn, PackageQuery query, SearchSemantics semantics) @@ -610,10 +686,15 @@ private static string MappedFieldCondition( } parameters.Add(MatchParameter(value, exact)); int paramNum = parameters.Count; - return $"EXISTS (SELECT 1 FROM {mapTableName} JOIN {tableName} ON " + - $"{mapTableName}.{valueName} = {tableName}.rowid " + - $"WHERE {mapTableName}.{mapOwnerColumn} = {rowidColumn} " + - $"AND {tableName}.{mapValueColumn} LIKE @p{paramNum})"; + // Use a non-correlated `rowid IN (...)` subquery rather than a correlated + // `EXISTS (... WHERE map.owner = packages.rowid ...)`. The correlated form + // re-scans the (un-indexed) map table once per candidate package row, which + // is quadratic: on the real winget index it takes ~19s under SQLite and + // effectively hangs under the Turso engine. The `IN` form evaluates the + // owner-id set a single time, collapsing the search back to milliseconds. + return $"{rowidColumn} IN (SELECT {mapTableName}.{mapOwnerColumn} FROM {mapTableName} " + + $"JOIN {tableName} ON {mapTableName}.{valueName} = {tableName}.rowid " + + $"WHERE {tableName}.{mapValueColumn} LIKE @p{paramNum})"; } private static string MatchParameter(string value, bool exact) diff --git a/dotnet/src/Devolutions.Pinget.PowerShell.Cmdlets/ModuleFiles/Devolutions.Pinget.Client.psd1 b/dotnet/src/Devolutions.Pinget.PowerShell.Cmdlets/ModuleFiles/Devolutions.Pinget.Client.psd1 index 2570407..ee8f831 100644 --- a/dotnet/src/Devolutions.Pinget.PowerShell.Cmdlets/ModuleFiles/Devolutions.Pinget.Client.psd1 +++ b/dotnet/src/Devolutions.Pinget.PowerShell.Cmdlets/ModuleFiles/Devolutions.Pinget.Client.psd1 @@ -1,6 +1,6 @@ @{ RootModule = 'Devolutions.Pinget.Client.psm1' - ModuleVersion = '0.8.0' + ModuleVersion = '0.8.1' CompatiblePSEditions = @('Desktop', 'Core') GUID = 'c6d1b5f2-5ccd-4771-9480-25caad7c58bd' Author = 'Devolutions' diff --git a/dotnet/src/Devolutions.Pinget.PowerShell.Engine/PowerShellEngineVersion.cs b/dotnet/src/Devolutions.Pinget.PowerShell.Engine/PowerShellEngineVersion.cs index ddae344..35746fb 100644 --- a/dotnet/src/Devolutions.Pinget.PowerShell.Engine/PowerShellEngineVersion.cs +++ b/dotnet/src/Devolutions.Pinget.PowerShell.Engine/PowerShellEngineVersion.cs @@ -2,5 +2,5 @@ namespace Devolutions.Pinget.PowerShell.Engine; public static class PowerShellEngineVersion { - public const string Current = "0.8.0"; + public const string Current = "0.8.1"; } diff --git a/nuget/Devolutions.Pinget.Cli.DotNet/Devolutions.Pinget.Cli.DotNet.csproj b/nuget/Devolutions.Pinget.Cli.DotNet/Devolutions.Pinget.Cli.DotNet.csproj index 0875594..d03f398 100644 --- a/nuget/Devolutions.Pinget.Cli.DotNet/Devolutions.Pinget.Cli.DotNet.csproj +++ b/nuget/Devolutions.Pinget.Cli.DotNet/Devolutions.Pinget.Cli.DotNet.csproj @@ -1,7 +1,7 @@ - 0.8.0 + 0.8.1 Devolutions Inc. Devolutions Devolutions.Pinget.Cli.DotNet diff --git a/nuget/Devolutions.Pinget.Cli.Rust/Devolutions.Pinget.Cli.Rust.csproj b/nuget/Devolutions.Pinget.Cli.Rust/Devolutions.Pinget.Cli.Rust.csproj index b992034..89cd0ee 100644 --- a/nuget/Devolutions.Pinget.Cli.Rust/Devolutions.Pinget.Cli.Rust.csproj +++ b/nuget/Devolutions.Pinget.Cli.Rust/Devolutions.Pinget.Cli.Rust.csproj @@ -1,7 +1,7 @@ - 0.8.0 + 0.8.1 Devolutions Inc. Devolutions Devolutions.Pinget.Cli.Rust diff --git a/rust/crates/pinget-cli/Cargo.toml b/rust/crates/pinget-cli/Cargo.toml index 7124030..6f243c9 100644 --- a/rust/crates/pinget-cli/Cargo.toml +++ b/rust/crates/pinget-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pinget-cli" -version = "0.8.0" +version = "0.8.1" edition = "2024" [lints] @@ -13,7 +13,7 @@ path = "src/main.rs" [dependencies] anyhow = "1.0.102" clap = { version = "4.6.1", features = ["derive"] } -pinget-core = { version = "0.8.0", path = "../pinget-core" } +pinget-core = { version = "0.8.1", path = "../pinget-core" } chrono = "0.4.44" dirs = "6.0" jsonschema = "0.30" diff --git a/rust/crates/pinget-com/Cargo.toml b/rust/crates/pinget-com/Cargo.toml index b8bc139..102dd18 100644 --- a/rust/crates/pinget-com/Cargo.toml +++ b/rust/crates/pinget-com/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pinget-com" -version = "0.8.0" +version = "0.8.1" edition = "2024" description = "Windows-only native COM bridge for Pinget backed by pinget-core." license = "MIT" diff --git a/rust/crates/pinget-core/Cargo.toml b/rust/crates/pinget-core/Cargo.toml index 1c4186a..6c74c28 100644 --- a/rust/crates/pinget-core/Cargo.toml +++ b/rust/crates/pinget-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pinget-core" -version = "0.8.0" +version = "0.8.1" edition = "2024" description = "Pure Rust Pinget core library that works directly with source caches, REST endpoints, and installed package state without COM." license = "MIT" diff --git a/rust/crates/pinget-core/src/lib.rs b/rust/crates/pinget-core/src/lib.rs index bf749fc..3bfe36c 100644 --- a/rust/crates/pinget-core/src/lib.rs +++ b/rust/crates/pinget-core/src/lib.rs @@ -2893,6 +2893,11 @@ impl Repository { )); fs::write(&temp_index_path, index_bytes).context("failed to persist temporary source index")?; replace_file(&temp_index_path, &index_path).context("failed to persist source index")?; + // The new index.db is a fresh database; any WAL/SHM sidecars + // left from the previous one no longer match it and would make + // SQLite recover against the wrong database. Drop them so the + // next open starts clean. + remove_sqlite_sidecars(&index_path); source.last_update = Some(Utc::now()); source.source_version = header_version; return Ok(format!("downloaded {}", candidate)); @@ -4296,8 +4301,22 @@ fn preindexed_index_path(app_root: &Path, source: &SourceRecord) -> PathBuf { source_state_dir(app_root, source).join("index.db") } +/// Removes the `-wal` and `-shm` sidecars that SQLite/Turso leaves next to a +/// database file. Used after replacing `index.db` so a stale write-ahead log +/// from the previous database is not replayed against the new one. +fn remove_sqlite_sidecars(db_path: &Path) { + for suffix in ["-wal", "-shm"] { + let mut sidecar = db_path.as_os_str().to_owned(); + sidecar.push(suffix); + let _ = fs::remove_file(PathBuf::from(sidecar)); + } +} + #[cfg(windows)] -fn replace_file(source: &Path, target: &Path) -> Result<()> { +const REPLACE_FILE_RETRY_ATTEMPTS: u32 = 5; + +#[cfg(windows)] +fn move_replace_existing(source: &Path, target: &Path) -> std::io::Result<()> { let source_wide = source .as_os_str() .encode_wide() @@ -4313,12 +4332,67 @@ fn replace_file(source: &Path, target: &Path) -> Result<()> { // live for the duration of the call; MoveFileExW does not retain them. let ok = unsafe { MoveFileExW(source_wide.as_ptr(), target_wide.as_ptr(), flags) }; if ok == 0 { - return Err(std::io::Error::last_os_error()) - .with_context(|| format!("failed to replace {} with {}", target.display(), source.display())); + return Err(std::io::Error::last_os_error()); } Ok(()) } +#[cfg(windows)] +fn replace_file(source: &Path, target: &Path) -> Result<()> { + let to_err = |error: std::io::Error| -> anyhow::Error { + anyhow::Error::new(error).context(format!( + "failed to replace {} with {}", + target.display(), + source.display() + )) + }; + + // Fast path: atomic replace. Succeeds whenever nothing holds the target open. + let first = match move_replace_existing(source, target) { + Ok(()) => return Ok(()), + Err(error) => error, + }; + + // A missing source or absent target is not a transient lock — fail fast + // instead of spinning through the retry/stage-aside path. + if !source.exists() || !target.exists() { + return Err(to_err(first)); + } + + // MOVEFILE_REPLACE_EXISTING is denied while another handle holds the target + // open. In practice that handle is an on-access AV scan of the freshly + // written file or a concurrent pinget process reading the index; both are + // short-lived, so retry briefly with a small backoff before falling back. + let mut last = first; + for attempt in 1..=REPLACE_FILE_RETRY_ATTEMPTS { + std::thread::sleep(std::time::Duration::from_millis(50 * u64::from(attempt))); + match move_replace_existing(source, target) { + Ok(()) => return Ok(()), + Err(error) => last = error, + } + } + + // Persistent holder: stage the existing index aside, then move the new file + // into the freed name. Renaming the existing file succeeds even while a + // reader keeps its handle, because SQLite/Turso opens with FILE_SHARE_DELETE. + // The staged copy is only removed once the new file is in place, and restored + // if the swap fails, so a working file is never lost. + let staged = target.with_extension("old.tmp"); + if move_replace_existing(target, &staged).is_err() { + return Err(to_err(last)); + } + match move_replace_existing(source, target) { + Ok(()) => { + let _ = fs::remove_file(&staged); + Ok(()) + } + Err(error) => { + let _ = move_replace_existing(&staged, target); + Err(to_err(error)) + } + } +} + #[cfg(not(windows))] fn replace_file(source: &Path, target: &Path) -> Result<()> { fs::rename(source, target) @@ -5638,11 +5712,17 @@ fn mapped_field_condition( }; params.push(match_parameter(value, exact)); let parameter = params.len(); + // Use a non-correlated `rowid IN (...)` subquery rather than a correlated + // `EXISTS (... WHERE map.owner = packages.rowid ...)`. The correlated form + // re-scans the (un-indexed) map table once per candidate package row, which + // is quadratic: on the real winget index it takes ~19s under SQLite and + // effectively hangs under the Turso engine. The `IN` form evaluates the + // owner-id set a single time, collapsing the search back to milliseconds. format!( - "EXISTS (SELECT 1 FROM {map_table_name} JOIN {table_name} ON \ + "{rowid_column} IN (SELECT {map_table_name}.{map_owner_column} \ + FROM {map_table_name} JOIN {table_name} ON \ {map_table_name}.{value_name} = {table_name}.rowid \ - WHERE {map_table_name}.{map_owner_column} = {rowid_column} \ - AND {table_name}.{map_value_column} LIKE ?{parameter})" + WHERE {table_name}.{map_value_column} LIKE ?{parameter})" ) } @@ -10807,6 +10887,26 @@ Installers: assert_eq!(params.len(), 5); } + #[test] + fn tag_and_command_conditions_are_non_correlated() { + // The tag/command map lookups must be emitted as non-correlated + // `rowid IN (SELECT ...)` subqueries. A correlated `EXISTS (... WHERE + // map.owner = packages.rowid ...)` re-scans the un-indexed map tables + // per package row, which is quadratic (~19s on the real winget index + // under SQLite, and an effective hang under Turso). Guard against a + // regression back to the correlated form. + let query = PackageQuery { + query: Some("terminal".to_owned()), + ..PackageQuery::default() + }; + + let (where_clause, _params) = build_preindexed_where_clause(&query, true, SearchSemantics::Many); + + assert!(where_clause.contains("packages.rowid IN (SELECT")); + assert!(!where_clause.contains("EXISTS")); + assert!(!where_clause.contains("= packages.rowid")); + } + #[test] fn show_query_single_omits_tag_and_command_conditions() { let query = PackageQuery { @@ -12290,6 +12390,35 @@ Installers: let _ = fs::remove_dir_all(root); } + #[cfg(windows)] + #[test] + fn replace_file_succeeds_while_target_is_open() { + use std::os::windows::fs::OpenOptionsExt; + // FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE — the sharing + // mode SQLite/Turso uses. An atomic MOVEFILE_REPLACE_EXISTING is denied + // while this handle is open, so the stage-aside fallback must take over. + const SHARE_ALL: u32 = 0x0000_0001 | 0x0000_0002 | 0x0000_0004; + + let root = temp_app_root("replace_file_open_target"); + fs::create_dir_all(&root).expect("create replace root"); + let target = root.join("index.db"); + let source = root.join("index.new.tmp"); + fs::write(&target, b"old-index").expect("write existing target"); + fs::write(&source, b"new-index").expect("write replacement"); + + let held = fs::OpenOptions::new() + .read(true) + .share_mode(SHARE_ALL) + .open(&target) + .expect("open target with share-delete"); + + replace_file(&source, &target).expect("stage-aside replace should succeed"); + + assert_eq!(fs::read(&target).expect("new index in place"), b"new-index"); + drop(held); + let _ = fs::remove_dir_all(root); + } + fn make_source_package(versions: &[&str]) -> Vec { let cursor = Cursor::new(Vec::new()); let mut archive = zip::ZipWriter::new(cursor);