From 95918414a0c27f67d2aa4faa5736fc491796db3d Mon Sep 17 00:00:00 2001 From: mika kuns Date: Fri, 24 Jul 2026 14:48:44 +0200 Subject: [PATCH] fix(installer): retry stashing app/worker through transient file locks on update The update pipeline moved app/ and worker/ to .bak before extraction with no retry, so a just-killed worker whose file handles had not been released yet (WaitForExit returns before the OS flushes them) caused Directory.Move to throw ERROR_SHARING_VIOLATION, stopping the pipeline on a cryptic error screen. - DownloadAndExtractStep: stash/rollback moves+deletes now retry through transient IO/access errors (~5s); a persistent lock returns an actionable message instead of the raw error. - StopWorkerStep: reading MainModule no longer skips Kill on failure, and a short settle follows the kill so handles are released before extraction. --- .../Steps/DownloadAndExtractStep.cs | 56 +++++++++++++++---- .../Steps/StopWorkerStep.cs | 15 ++++- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/ClaudeDo.Installer/Steps/DownloadAndExtractStep.cs b/src/ClaudeDo.Installer/Steps/DownloadAndExtractStep.cs index 515c54dc..a2cef139 100644 --- a/src/ClaudeDo.Installer/Steps/DownloadAndExtractStep.cs +++ b/src/ClaudeDo.Installer/Steps/DownloadAndExtractStep.cs @@ -1,5 +1,6 @@ using System.IO; using System.IO.Compression; +using System.Threading; using ClaudeDo.Installer.Core; using ClaudeDo.Releases; @@ -77,10 +78,23 @@ public sealed class DownloadAndExtractStep : IInstallStep var appBak = appDest + ".bak"; var workerBak = workerDest + ".bak"; - if (Directory.Exists(appBak)) Directory.Delete(appBak, recursive: true); - if (Directory.Exists(workerBak)) Directory.Delete(workerBak, recursive: true); - if (Directory.Exists(appDest)) Directory.Move(appDest, appBak); - if (Directory.Exists(workerDest)) Directory.Move(workerDest, workerBak); + try + { + if (Directory.Exists(appBak)) DeleteWithRetry(appBak); + if (Directory.Exists(workerBak)) DeleteWithRetry(workerBak); + if (Directory.Exists(appDest)) MoveWithRetry(appDest, appBak); + if (Directory.Exists(workerDest)) MoveWithRetry(workerDest, workerBak); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // A just-stopped app/worker (or an Explorer/terminal window sitting in + // the install dir) still held a handle. Surface an actionable message + // instead of the raw "process cannot access the file" error. + return StepResult.Fail( + "Could not replace the existing app/worker files — they are still in use. " + + "Make sure ClaudeDo is fully closed (app and worker) and no Explorer or " + + $"terminal window is open inside the install folder, then run the update again. Details: {ex.Message}"); + } progress.Report("Extracting..."); Directory.CreateDirectory(ctx.InstallDirectory); @@ -91,17 +105,17 @@ public sealed class DownloadAndExtractStep : IInstallStep catch (Exception ex) { // Roll back to previous binaries. - if (Directory.Exists(appDest)) Directory.Delete(appDest, recursive: true); - if (Directory.Exists(workerDest)) Directory.Delete(workerDest, recursive: true); - if (Directory.Exists(appBak)) Directory.Move(appBak, appDest); - if (Directory.Exists(workerBak)) Directory.Move(workerBak, workerDest); + if (Directory.Exists(appDest)) DeleteWithRetry(appDest); + if (Directory.Exists(workerDest)) DeleteWithRetry(workerDest); + if (Directory.Exists(appBak)) MoveWithRetry(appBak, appDest); + if (Directory.Exists(workerBak)) MoveWithRetry(workerBak, workerDest); return StepResult.Fail( $"Extraction failed; previous binaries have been restored: {ex.Message}."); } // Success — drop stash. - if (Directory.Exists(appBak)) Directory.Delete(appBak, recursive: true); - if (Directory.Exists(workerBak)) Directory.Delete(workerBak, recursive: true); + if (Directory.Exists(appBak)) DeleteWithRetry(appBak); + if (Directory.Exists(workerBak)) DeleteWithRetry(workerBak); ctx.InstalledVersion = release.TagName.TrimStart('v', 'V'); return StepResult.Ok(); @@ -111,4 +125,26 @@ public sealed class DownloadAndExtractStep : IInstallStep try { Directory.Delete(scratchDir, recursive: true); } catch { /* best effort */ } } } + + private static void MoveWithRetry(string source, string dest) + => RetryIo(() => Directory.Move(source, dest)); + + private static void DeleteWithRetry(string dir) + => RetryIo(() => Directory.Delete(dir, recursive: true)); + + // WaitForExit returns before Windows releases a just-killed process's file + // handles, so the stash Move/Delete can briefly hit a sharing violation. + // Retry through transient IO/access errors (~5s) before letting it surface. + private static void RetryIo(Action action) + { + const int attempts = 10; + for (var i = 0; ; i++) + { + try { action(); return; } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException && i < attempts - 1) + { + Thread.Sleep(500); + } + } + } } diff --git a/src/ClaudeDo.Installer/Steps/StopWorkerStep.cs b/src/ClaudeDo.Installer/Steps/StopWorkerStep.cs index e0041530..02075db3 100644 --- a/src/ClaudeDo.Installer/Steps/StopWorkerStep.cs +++ b/src/ClaudeDo.Installer/Steps/StopWorkerStep.cs @@ -19,22 +19,33 @@ public sealed class StopWorkerStep : IInstallStep { progress.Report("Stopping ClaudeDo processes (if running)..."); var installDir = ctx.InstallDirectory; + var killedAny = false; foreach (var name in ProcessNames) { foreach (var p in Process.GetProcessesByName(name)) { try { - var path = p.MainModule?.FileName; + // Scope to THIS install when the module path is readable; if it + // can't be read (access race / exiting process), fall through and + // kill anyway — a survivor would lock the install dir during + // extraction. Reading MainModule must not skip the Kill. + string? path = null; + try { path = p.MainModule?.FileName; } catch { /* unreadable — kill anyway */ } if (path is not null && !IsUnder(path, installDir)) continue; p.Kill(entireProcessTree: true); p.WaitForExit(10000); + killedAny = true; } catch { /* process may have exited or be inaccessible */ } finally { p.Dispose(); } } } - await Task.CompletedTask; + + // WaitForExit returns before the OS releases the process's file handles. + // Give it a moment so DownloadAndExtractStep's Directory.Move doesn't race + // a still-open handle. (That step also retries, this just avoids the churn.) + if (killedAny) await Task.Delay(1500, ct); return StepResult.Ok(); }