mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(windows): make the install directory actually replaceable before updating (#403)
* fix(windows): make the install directory actually replaceable before updating
The updater stopped the daemon and started the Inno installer the moment
the daemon's endpoint disappeared — but the endpoint going away is not
the same event as the images being released. The ConPTY hosts
(OpenConsole.exe) are the daemon's children, not the shells', so the
per-pane kill never reached them, and the daemon's exit(0) skipped every
destructor that would have closed them; they kept the installed
OpenConsole.exe open for seconds after --stop-daemon returned. Silent
Setup then hit the lock, took the suppressed dialog's default (Abort),
and the updater's recovery relaunched the old build — "updated,
restarted, still the old version". A daemon that died without cleaning
up made it permanent: its orphaned hosts survive indefinitely, which is
the DeleteFile-code-5 users hit even after "closing everything".
Reproduced both shapes in isolation before fixing: with a pane open,
--stop-daemon returned ~1s in while OpenConsole.exe stayed locked for
another ~1.4s; after taskkill on the daemon, the orphaned host held the
lock forever.
The shutdown now finishes what it starts, at every layer that can be
the last one standing:
* The daemon reaps its remaining descendants and waits for them
before exiting, while the endpoint — the signal stop() watches —
is still up.
* stop() reads the pidfile before asking, and waits for that process
to actually exit after the endpoint goes, not just stop listening.
* The recorded-daemon reap waits for the images to be released
instead of returning on the async TerminateProcess.
* stop_for_update(dir) — reached via --stop-daemon
--update-install-dir, which PrepareToInstall and the portable
updater now pass — also terminates anything still running from the
installation directory (the orphan case no pidfile can name) and
only returns once the .exe/.dll images there open for writing,
naming the holdouts in the error if they never do.
* The updater runs that clearing itself before invoking Setup, so a
directory that cannot be cleared fails with a cause in update.log
and relaunches the previous build, instead of Inno's bare
"DeleteFile failed; code 5".
The update dialog on Windows also told a macOS truth — "the background
service keeps running, so whatever is open in your panes survives".
Windows cannot replace a running daemon's image, so its install path
stops the service; the dialog now says so.
* fix(windows): tighten the install-dir clearing per review
- An image that fails to canonicalize stays in the lock check instead of
being silently skipped; only a positive match against the caller's own
running image is excluded.
- reap_recorded_daemon shares one deadline across the whole tree via a
new winproc::terminate_and_wait_all, which stop_for_update and
reap_descendants_of now use too — one implementation of "terminate,
then wait, bounded overall" instead of three.
- [UninstallRun] passes --update-install-dir "{app}" like
PrepareToInstall, so uninstalling after a daemon crash gets the same
orphaned-ConPTY-host cleanup as upgrading.
* fix(update): close three gaps the update audit found
- macOS updater: wait for the parent by watching getppid() reparent to
launchd instead of polling kill(pid, 0), which a recycled pid could
satisfy forever. The kill loop remains only for a hand-run updater.
- Windows: a new update guard (config-dir update.lock, held by the
updater from daemon stop to relaunch) makes ensure_running refuse to
spawn a daemon mid-install, so a tty7 CLI call or manual launch can no
longer relock the images the installer is replacing. Stale guards —
dead writer or past the TTL — are shed on sight.
- Windows portable: the update backup now carries an incomplete marker
from before the first file moves until the replacement lands. At
launch the app reports a backup still carrying it as an interrupted
update (the installation may mix two versions; the old files are
preserved), and silently removes marker-less backups a finished
update failed to delete past an antivirus hold.
* fix(update): verify the guard's writer by start time, and guard manual Setup runs
Review round three, both findings and all three minors:
- The guard no longer expires a live, verified holder: a pid is believed
to be the writer only if the process behind it started before the
guard was written (winproc::creation_time via GetProcessTimes), which
is what tells a genuine holder from a recycled pid. The TTL now bounds
only the unverifiable case, so an install slowed past ten minutes by
an antivirus sweep keeps its protection.
- Manual Setup runs get the guard too: the --stop-daemon
--update-install-dir helper holds it in its parent's name — the Setup
or uninstaller that keeps replacing files after the helper returns —
and it goes stale when that parent exits. ensure_running gained five
seconds of patience so the post-install "Launch tty7" click, racing
Setup's own exit, gets its daemon instead of an error.
- processes_running_from also matches images against the canonicalized
install-dir spelling (junction, subst, 8.3 given form).
- reconcile_portable_backups reports every interrupted backup, not the
first.
- The unix signal-and-wait loop now reuses wait_for_recorded_exit.
* style: rustfmt
---------
Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
This commit is contained in:
@@ -130,12 +130,16 @@ Filename: "{app}\tty7-app.exe"; Description: "{cm:LaunchProgram,tty7}"; Flags: n
|
||||
|
||||
[UninstallRun]
|
||||
; Stop the daemon before the uninstaller deletes tty7-app.exe — the running daemon
|
||||
; is the locked image of that file, so removing it fails otherwise. This runs at
|
||||
; the start of uninstallation, before any files are removed. The installed binary
|
||||
; is this version, which understands the flag; runhidden suppresses any flash and
|
||||
; the call returns without opening a window. RunOnceId keys the entry so a repeated
|
||||
; uninstall doesn't run it twice.
|
||||
Filename: "{app}\tty7-app.exe"; Parameters: "--stop-daemon"; Flags: runhidden waituntilterminated; RunOnceId: "StopDaemon"
|
||||
; is the locked image of that file, so removing it fails otherwise. Naming {app}
|
||||
; widens the stop the same way PrepareToInstall's does: ConPTY hosts orphaned by
|
||||
; a daemon that never got to shut down keep the installed images open, and their
|
||||
; DeleteFile fails an uninstall exactly as it fails an upgrade. (The stop excludes
|
||||
; the calling process itself, so the binary running this step is safe.) This runs
|
||||
; at the start of uninstallation, before any files are removed. The installed
|
||||
; binary is this version, which understands the flags; runhidden suppresses any
|
||||
; flash and the call returns without opening a window. RunOnceId keys the entry
|
||||
; so a repeated uninstall doesn't run it twice.
|
||||
Filename: "{app}\tty7-app.exe"; Parameters: "--stop-daemon --update-install-dir ""{app}"""; Flags: runhidden waituntilterminated; RunOnceId: "StopDaemon"
|
||||
; Unconditional, and deliberately not gated on the task: an install that had the
|
||||
; menu registered and was later upgraded without the box ticked still holds the
|
||||
; keys, and verbs pointing at a deleted exe are worse than a no-op. Removing keys
|
||||
@@ -148,14 +152,20 @@ Filename: "{app}\tty7-app.exe"; Parameters: "--unregister-explorer-menu"; Flags:
|
||||
understand --stop-daemon and would launch the GUI instead — so we extract the
|
||||
*new* tty7-app.exe to {tmp} and run that. It connects to the running daemon, hangs
|
||||
up every shell, waits for it to exit (releasing the file lock), then returns
|
||||
without opening a window. Best effort: any failure falls through to the Restart
|
||||
Manager backstop, and a fresh install simply has no daemon to stop. *)
|
||||
without opening a window. Naming {app} widens the stop into "make this directory
|
||||
replaceable": ConPTY hosts (OpenConsole.exe) orphaned by a daemon that never got
|
||||
to shut down keep the installed images open — invisible to the daemon stop, fatal
|
||||
to the DeleteFile below — so anything still running from {app} is terminated and
|
||||
the call waits until the images there actually open for writing. Best effort: any
|
||||
failure falls through to the Restart Manager backstop, and a fresh install simply
|
||||
has nothing to stop. *)
|
||||
function PrepareToInstall(var NeedsRestart: Boolean): String;
|
||||
var
|
||||
ResultCode: Integer;
|
||||
begin
|
||||
ExtractTemporaryFile('tty7-app.exe');
|
||||
Exec(ExpandConstant('{tmp}\tty7-app.exe'), '--stop-daemon', '',
|
||||
Exec(ExpandConstant('{tmp}\tty7-app.exe'),
|
||||
'--stop-daemon --update-install-dir "' + ExpandConstant('{app}') + '"', '',
|
||||
SW_HIDE, ewWaitUntilTerminated, ResultCode);
|
||||
Result := '';
|
||||
end;
|
||||
|
||||
Reference in New Issue
Block a user