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.
This commit is contained in:
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user