Skip to content

Commit 2187aa4

Browse files
GabrielDufGabrielDuf
authored
Prevent crashes from partial in-place updates (#4868)
1 parent 4a57234 commit 2187aa4

6 files changed

Lines changed: 253 additions & 10 deletions

File tree

‎UniGetUI.iss‎

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ DefaultDirName="{autopf64}\UniGetUI"
2929
DisableProgramGroupPage=yes
3030
DisableDirPage=no
3131
DirExistsWarning=no
32-
CloseApplications=no
32+
; Force-close any process holding files we overwrite (backstop for the kill in PrepareToInstall).
33+
CloseApplications=force
34+
CloseApplicationsFilter=*.exe,*.dll
35+
RestartApplications=no
3336
; Default to per-user install mode and let the dialog opt into all-users installs when needed.
3437
PrivilegesRequired=lowest
3538
PrivilegesRequiredOverridesAllowed=dialog
@@ -97,19 +100,70 @@ begin
97100
WizardForm.Bevel1.Visible := True;
98101
end;
99102
100-
procedure TaskKill(FileName: String);
103+
// Kills all instances of an image and loops until none remain (taskkill returns 0 while killing, 128 when none left).
104+
procedure TaskKillWait(FileName: String);
101105
var
102-
ResultCode: Integer;
106+
ResultCode, Attempts: Integer;
103107
begin
104-
Exec('taskkill.exe', '/f /im ' + '"' + FileName + '"', '', SW_HIDE,
105-
ewWaitUntilTerminated, ResultCode);
108+
Attempts := 0;
109+
repeat
110+
if not Exec('taskkill.exe', '/f /im "' + FileName + '"', '', SW_HIDE,
111+
ewWaitUntilTerminated, ResultCode) then
112+
Break;
113+
if ResultCode <> 0 then
114+
Break;
115+
Sleep(500);
116+
Attempts := Attempts + 1;
117+
until Attempts >= 10;
106118
end;
107119
108120
procedure KillRunningApps;
109121
begin
110-
TaskKill('WingetUI.exe');
111-
TaskKill('UniGetUI.exe');
112-
TaskKill('UniGetUI.Avalonia.exe');
122+
TaskKillWait('WingetUI.exe');
123+
TaskKillWait('UniGetUI.exe');
124+
TaskKillWait('UniGetUI.Avalonia.exe');
125+
// Elevator (gsudo cache) and pinget live in {app} and lock their own files.
126+
TaskKillWait('UniGetUI Elevator.exe');
127+
TaskKillWait('pinget.exe');
128+
Sleep(1000); // let the OS release file handles before copying
129+
130+
end;
131+
132+
function GetCurrentProcessId: Cardinal; external 'GetCurrentProcessId@kernel32.dll stdcall';
133+
134+
function UpdateMarkerPath(): String;
135+
begin
136+
Result := ExpandConstant('{app}\.unigetui-update-in-progress');
137+
end;
138+
139+
// Marker holds our PID; the app blocks only while this installer runs. Name MUST match UpdateInProgressGuard.MarkerFileName.
140+
procedure WriteUpdateMarker;
141+
var
142+
Pid: Int64;
143+
begin
144+
ForceDirectories(ExpandConstant('{app}'));
145+
Pid := GetCurrentProcessId;
146+
SaveStringToFile(UpdateMarkerPath(), IntToStr(Pid), False);
147+
end;
148+
149+
procedure RemoveUpdateMarker;
150+
begin
151+
DeleteFile(UpdateMarkerPath());
152+
end;
153+
154+
// Runs before any file is copied: shut everything down, then mark the copy window.
155+
function PrepareToInstall(var NeedsRestart: Boolean): String;
156+
begin
157+
KillRunningApps;
158+
WriteUpdateMarker;
159+
Result := '';
160+
end;
161+
162+
// Clear the marker once the copy is done, before the post-install launch.
163+
procedure CurStepChanged(CurStep: TSetupStep);
164+
begin
165+
if CurStep = ssPostInstall then
166+
RemoveUpdateMarker;
113167
end;
114168
115169
function CmdLineParamExists(const Value: string): Boolean;
@@ -132,6 +186,8 @@ procedure ExitProcess(exitCode:integer);
132186
133187
procedure DeinitializeSetup();
134188
begin
189+
RemoveUpdateMarker; // also clear on abort, before ssPostInstall
190+
135191
if (CustomExitCode <> 0) then
136192
begin
137193
DelTree(ExpandConstant('{tmp}'), True, True, True);
@@ -215,8 +271,8 @@ Root: HKA; Subkey: "Software\Classes\UniGetUI.PackageBundle\shell\open\command";
215271
Source: "{srcexe}"; DestDir: "{app}"; DestName: "UniGetUI.Installer.exe"; Flags: external ignoreversion; Tasks: regularinstall; Check: not CmdLineParamExists('/NoDeployInstaller');
216272
; Deploy integrity tree
217273
Source: "unigetui_bin\IntegrityTree.json"; DestDir: "{app}"; Flags: createallsubdirs ignoreversion recursesubdirs;
218-
; Deploy executable files
219-
Source: "unigetui_bin\{#MyAppExeName}"; DestDir: "{app}"; Flags: ignoreversion; BeforeInstall: KillRunningApps;
274+
; Deploy executable files (running instances already killed in PrepareToInstall).
275+
Source: "unigetui_bin\{#MyAppExeName}"; DestDir: "{app}"; Flags: ignoreversion;
220276
Source: "unigetui_bin\*"; DestDir: "{app}"; Flags: createallsubdirs ignoreversion recursesubdirs;
221277
; Make installation portable (if required)
222278
Source: "InstallerExtras\ForceUniGetUIPortable"; DestDir: "{app}"; Tasks: portableinstall

‎src/UniGetUI.Avalonia/Program.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using Avalonia;
33
using UniGetUI.Avalonia.Infrastructure;
4+
using UniGetUI.Core.Data;
45

56
namespace UniGetUI.Avalonia;
67

@@ -12,6 +13,17 @@ sealed class Program
1213
[STAThread]
1314
public static void Main(string[] args)
1415
{
16+
// Bail out if the installer is mid-swap (try/catch so the guard never blocks a normal launch).
17+
try
18+
{
19+
if (UpdateInProgressGuard.IsUpdateInProgress())
20+
{
21+
Environment.ExitCode = 0;
22+
return;
23+
}
24+
}
25+
catch { }
26+
1527
AvaloniaAppHost.Run(args);
1628
}
1729

‎src/UniGetUI.Core.Data.Tests/UpdateInProgressGuardTests.cs‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
namespace UniGetUI.Core.Data.Tests
2+
{
3+
public class UpdateInProgressGuardTests : IDisposable
4+
{
5+
// {root}/app stands in for {app}; {root} is its always-empty parent.
6+
private readonly string _root;
7+
private readonly string _appDir;
8+
9+
private static readonly Func<int, bool> ProcessAlive = _ => true;
10+
private static readonly Func<int, bool> ProcessDead = _ => false;
11+
12+
public UpdateInProgressGuardTests()
13+
{
14+
_root = Path.Combine(Path.GetTempPath(), "ugui-guard-" + Guid.NewGuid().ToString("N"));
15+
_appDir = Path.Combine(_root, "app");
16+
Directory.CreateDirectory(_appDir);
17+
}
18+
19+
public void Dispose()
20+
{
21+
try { Directory.Delete(_root, recursive: true); }
22+
catch { }
23+
}
24+
25+
private static string WriteMarker(string directory, string content = "1234")
26+
{
27+
Directory.CreateDirectory(directory);
28+
string path = Path.Combine(directory, UpdateInProgressGuard.MarkerFileName);
29+
File.WriteAllText(path, content);
30+
return path;
31+
}
32+
33+
[Fact]
34+
public void NoMarker_ReturnsFalse()
35+
{
36+
Assert.False(UpdateInProgressGuard.IsUpdateInProgress(_appDir, ProcessAlive));
37+
}
38+
39+
[Fact]
40+
public void MarkerWithRunningInstaller_ReturnsTrue()
41+
{
42+
WriteMarker(_appDir);
43+
Assert.True(UpdateInProgressGuard.IsUpdateInProgress(_appDir, ProcessAlive));
44+
}
45+
46+
[Fact]
47+
public void MarkerInParentWithRunningInstaller_ReturnsTrue()
48+
{
49+
// Avalonia runs from {app}\Avalonia; marker is in {app}.
50+
WriteMarker(_appDir);
51+
string child = Path.Combine(_appDir, "Avalonia");
52+
Directory.CreateDirectory(child);
53+
54+
Assert.True(UpdateInProgressGuard.IsUpdateInProgress(child, ProcessAlive));
55+
}
56+
57+
[Fact]
58+
public void MarkerWithDeadInstaller_ReturnsFalseAndIsDeleted()
59+
{
60+
string marker = WriteMarker(_appDir);
61+
62+
Assert.False(UpdateInProgressGuard.IsUpdateInProgress(_appDir, ProcessDead));
63+
Assert.False(File.Exists(marker)); // stale marker is cleaned up
64+
}
65+
66+
[Fact]
67+
public void MarkerWithUnreadableContent_ReturnsFalseAndIsKept()
68+
{
69+
string marker = WriteMarker(_appDir, "not-a-pid");
70+
71+
Assert.False(UpdateInProgressGuard.IsUpdateInProgress(_appDir, ProcessAlive));
72+
Assert.True(File.Exists(marker)); // not deleted: could be a partial write
73+
}
74+
75+
[Fact]
76+
public void RealProcessCheck_TreatsCurrentProcessAsRunning()
77+
{
78+
// Exercises the real IsProcessRunning via the parameterless overload.
79+
WriteMarker(_appDir, Environment.ProcessId.ToString());
80+
Assert.True(UpdateInProgressGuard.IsUpdateInProgress(_appDir));
81+
}
82+
83+
[Fact]
84+
public void MarkerFileName_MatchesInstallerContract()
85+
{
86+
Assert.Equal(".unigetui-update-in-progress", UpdateInProgressGuard.MarkerFileName);
87+
}
88+
}
89+
}

‎src/UniGetUI.Core.Data/InternalsVisibleTo.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
using System.Runtime.CompilerServices;
2+
3+
[assembly: InternalsVisibleTo("UniGetUI.Core.Data.Tests")]

‎src/UniGetUI.Core.Data/UpdateInProgressGuard.cs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
using System.Diagnostics;
2+
3+
namespace UniGetUI.Core.Data
4+
{
5+
// Blocks UI startup while the Windows installer is replacing files in {app} (see UniGetUI.iss),
6+
// so an instance launched mid-update doesn't load a half-written binary set and crash.
7+
public static class UpdateInProgressGuard
8+
{
9+
// MUST match the marker name written by UniGetUI.iss. The file holds the installer's PID.
10+
public const string MarkerFileName = ".unigetui-update-in-progress";
11+
12+
public static bool IsUpdateInProgress()
13+
{
14+
if (!OperatingSystem.IsWindows())
15+
return false;
16+
17+
return IsUpdateInProgress(AppContext.BaseDirectory);
18+
}
19+
20+
// Checks the running dir and its parent (the Avalonia UI runs from {app}\Avalonia).
21+
internal static bool IsUpdateInProgress(string baseDirectory)
22+
=> IsUpdateInProgress(baseDirectory, IsProcessRunning);
23+
24+
internal static bool IsUpdateInProgress(string baseDirectory, Func<int, bool> isProcessRunning)
25+
{
26+
try
27+
{
28+
if (MarkerIsActive(baseDirectory, isProcessRunning))
29+
return true;
30+
31+
string? parent = Directory
32+
.GetParent(baseDirectory.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar))
33+
?.FullName;
34+
return parent is not null && MarkerIsActive(parent, isProcessRunning);
35+
}
36+
catch
37+
{
38+
return false;
39+
}
40+
}
41+
42+
// Active only while the installer that wrote the PID is still running, so the guard tracks
43+
// the real copy window (any duration) and self-heals if the installer dies without cleanup.
44+
private static bool MarkerIsActive(string directory, Func<int, bool> isProcessRunning)
45+
{
46+
string marker = Path.Combine(directory, MarkerFileName);
47+
if (!File.Exists(marker))
48+
return false;
49+
50+
if (!int.TryParse(File.ReadAllText(marker).Trim(), out int pid))
51+
return false; // unreadable (e.g. racing the installer's write) — leave it alone
52+
53+
if (isProcessRunning(pid))
54+
return true;
55+
56+
try { File.Delete(marker); } catch { /* stale: installer is gone */ }
57+
return false;
58+
}
59+
60+
private static bool IsProcessRunning(int pid)
61+
{
62+
if (pid <= 0)
63+
return false;
64+
65+
try
66+
{
67+
using var process = Process.GetProcessById(pid);
68+
return !process.HasExited;
69+
}
70+
catch (ArgumentException)
71+
{
72+
return false;
73+
}
74+
}
75+
}
76+
}

‎src/UniGetUI/EntryPoint.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ private static void Main(string[] args)
5151
Environment.ExitCode = WinUiHeadlessHost.RunAsync(args).GetAwaiter().GetResult();
5252
return;
5353
}
54+
else if (UpdateInProgressGuard.IsUpdateInProgress())
55+
{
56+
// Update is replacing install files; the installer relaunches us when done.
57+
Logger.Warn("An update is replacing install files; aborting UI startup until it completes.");
58+
Environment.ExitCode = 0;
59+
return;
60+
}
5461
else if (!ModernAppLauncher.IsClassicModeEnabled())
5562
{
5663
ModernAppLauncher.Launch(args);

0 commit comments

Comments
 (0)