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;
|
||||||
using System.IO.Compression;
|
using System.IO.Compression;
|
||||||
|
using System.Threading;
|
||||||
using ClaudeDo.Installer.Core;
|
using ClaudeDo.Installer.Core;
|
||||||
using ClaudeDo.Releases;
|
using ClaudeDo.Releases;
|
||||||
|
|
||||||
@@ -77,10 +78,23 @@ public sealed class DownloadAndExtractStep : IInstallStep
|
|||||||
var appBak = appDest + ".bak";
|
var appBak = appDest + ".bak";
|
||||||
var workerBak = workerDest + ".bak";
|
var workerBak = workerDest + ".bak";
|
||||||
|
|
||||||
if (Directory.Exists(appBak)) Directory.Delete(appBak, recursive: true);
|
try
|
||||||
if (Directory.Exists(workerBak)) Directory.Delete(workerBak, recursive: true);
|
{
|
||||||
if (Directory.Exists(appDest)) Directory.Move(appDest, appBak);
|
if (Directory.Exists(appBak)) DeleteWithRetry(appBak);
|
||||||
if (Directory.Exists(workerDest)) Directory.Move(workerDest, workerBak);
|
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...");
|
progress.Report("Extracting...");
|
||||||
Directory.CreateDirectory(ctx.InstallDirectory);
|
Directory.CreateDirectory(ctx.InstallDirectory);
|
||||||
@@ -91,17 +105,17 @@ public sealed class DownloadAndExtractStep : IInstallStep
|
|||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
// Roll back to previous binaries.
|
// Roll back to previous binaries.
|
||||||
if (Directory.Exists(appDest)) Directory.Delete(appDest, recursive: true);
|
if (Directory.Exists(appDest)) DeleteWithRetry(appDest);
|
||||||
if (Directory.Exists(workerDest)) Directory.Delete(workerDest, recursive: true);
|
if (Directory.Exists(workerDest)) DeleteWithRetry(workerDest);
|
||||||
if (Directory.Exists(appBak)) Directory.Move(appBak, appDest);
|
if (Directory.Exists(appBak)) MoveWithRetry(appBak, appDest);
|
||||||
if (Directory.Exists(workerBak)) Directory.Move(workerBak, workerDest);
|
if (Directory.Exists(workerBak)) MoveWithRetry(workerBak, workerDest);
|
||||||
return StepResult.Fail(
|
return StepResult.Fail(
|
||||||
$"Extraction failed; previous binaries have been restored: {ex.Message}.");
|
$"Extraction failed; previous binaries have been restored: {ex.Message}.");
|
||||||
}
|
}
|
||||||
|
|
||||||
// Success — drop stash.
|
// Success — drop stash.
|
||||||
if (Directory.Exists(appBak)) Directory.Delete(appBak, recursive: true);
|
if (Directory.Exists(appBak)) DeleteWithRetry(appBak);
|
||||||
if (Directory.Exists(workerBak)) Directory.Delete(workerBak, recursive: true);
|
if (Directory.Exists(workerBak)) DeleteWithRetry(workerBak);
|
||||||
|
|
||||||
ctx.InstalledVersion = release.TagName.TrimStart('v', 'V');
|
ctx.InstalledVersion = release.TagName.TrimStart('v', 'V');
|
||||||
return StepResult.Ok();
|
return StepResult.Ok();
|
||||||
@@ -111,4 +125,26 @@ public sealed class DownloadAndExtractStep : IInstallStep
|
|||||||
try { Directory.Delete(scratchDir, recursive: true); } catch { /* best effort */ }
|
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)...");
|
progress.Report("Stopping ClaudeDo processes (if running)...");
|
||||||
var installDir = ctx.InstallDirectory;
|
var installDir = ctx.InstallDirectory;
|
||||||
|
var killedAny = false;
|
||||||
foreach (var name in ProcessNames)
|
foreach (var name in ProcessNames)
|
||||||
{
|
{
|
||||||
foreach (var p in Process.GetProcessesByName(name))
|
foreach (var p in Process.GetProcessesByName(name))
|
||||||
{
|
{
|
||||||
try
|
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;
|
if (path is not null && !IsUnder(path, installDir)) continue;
|
||||||
p.Kill(entireProcessTree: true);
|
p.Kill(entireProcessTree: true);
|
||||||
p.WaitForExit(10000);
|
p.WaitForExit(10000);
|
||||||
|
killedAny = true;
|
||||||
}
|
}
|
||||||
catch { /* process may have exited or be inaccessible */ }
|
catch { /* process may have exited or be inaccessible */ }
|
||||||
finally { p.Dispose(); }
|
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();
|
return StepResult.Ok();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user