amnezia-vpn/amnezia-client

Fork 0

mirror of https://github.com/amnezia-vpn/amnezia-client.git synced 2026-10-15 22:02:03 +00:00

[PR #3252] fix(installer): do not treat noisy output of the package-manager check as "server busy" #3148

New issue

Open

opened 2026-10-14 21:07:50 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-14 21:07:50 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/amnezia-vpn/amnezia-client/pull/3252
Author: @ChesterFishmansWork
Created: 10/2/2026
Status: 🔄 Open

Base: dev ← Head: fix/3232-busy-check-false-positive

📝 Commits (8)

96b2cf6 fix(installer): never treat noisy output as "package manager busy"
33db6df fix(installer): do not trust a stale pid file or a timed-out lock check
dbfed0c docs(installer): say what the SERVER_BUSY marker now covers
7222c74 fix(installer): never treat noisy output as "package manager busy"
83659e5 fix(installer): never treat noisy output as "package manager busy"
67cd37d fix(installer): keep the package-manager check honest without psmisc or a working sudo
58d9ed4 fix(installer): make the lock check immune to symlinked lock paths
77dabc9 fix(installer): never answer SERVER_FREE with a blind lock scan

📊 Changes

2 files changed (+156 additions, -17 deletions)

View changed files

📝 client/core/controllers/selfhosted/installController.cpp (+44 -10)
📝 client/server_scripts/check_server_is_busy.sh (+112 -7)

📄 Description

Problem

InstallController::isServerDpkgBusy() treated any non-empty output of check_server_is_busy.sh (stdout and stderr) as "the package manager is busy" and retried up to 30 times with 10 s sleeps — i.e. a free server could make the app appear to hang for up to 5 minutes in the middle of a server update:

if (stdOut.isEmpty()) {
    return ErrorCode::NoError;   // free
} else {
    emit serverIsBusy(true);
    QThread::msleep(10000);      // retry, whatever the output was
}

The script itself was equally fragile: it decided "busy" from a non-empty output of fuser/cat rather than from their exit status.

Reported in #3232 (client on Fedora 43, server on Debian 12: changing Xray settings hangs on the check_server_is_busy.sh step).

Change

client/server_scripts/check_server_is_busy.sh

decides by exit code (fuser -s), wrapped in timeout 15 when available, so a stuck check cannot block the ssh channel;
prints the explicit marker SERVER_BUSY only when the lock is really held, and stays silent otherwise;
keeps the legacy strings (Packet manager not found, fuser not installed, cat not installed) for backward compatibility;
uses command -v instead of which || command -v duplication.

client/core/controllers/selfhosted/installController.cpp

busy is reported only when the output contains SERVER_BUSY;
otherwise the result of runScript() is propagated, so an ssh error is no longer silently treated as "server is free";
the 10 s retry sleep is split into 100 ms steps, so cancelling the installation is honoured immediately;
stale comment fixed (30 attempts, not 100).

Limitations (important)

The root cause was not reproduced locally — there is no reproduction environment. The change is based on static analysis of the code path and is verified statically only; no build/test run (Qt toolchain is not available in my environment).
An independent read-only review found no blocking issues. Non-blocking notes: if the server is genuinely busy but the marker is not printed, the failure surfaces later in the installation flow instead of being retried here — that trade-off is deliberate.
Related, but out of scope for this PR: other places that can make an ssh step wait indefinitely (SshSession::runScript, missing SSH_OPTIONS_TIMEOUT, stderr read after stdout).

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/amnezia-vpn/amnezia-client/pull/3252 **Author:** [@ChesterFishmansWork](https://github.com/ChesterFishmansWork) **Created:** 10/2/2026 **Status:** 🔄 Open **Base:** `dev` ← **Head:** `fix/3232-busy-check-false-positive` --- ### 📝 Commits (8) - [`96b2cf6`](https://github.com/amnezia-vpn/amnezia-client/commit/96b2cf612fc9dc6e874ea85f1f7dfb64a82cb553) fix(installer): never treat noisy output as "package manager busy" - [`33db6df`](https://github.com/amnezia-vpn/amnezia-client/commit/33db6df32bcaec0dfdc27aee0926b6d57685417a) fix(installer): do not trust a stale pid file or a timed-out lock check - [`dbfed0c`](https://github.com/amnezia-vpn/amnezia-client/commit/dbfed0c9cdf68bf3862bbf4a3b14268ad11aa704) docs(installer): say what the SERVER_BUSY marker now covers - [`7222c74`](https://github.com/amnezia-vpn/amnezia-client/commit/7222c742d111165ba78897856afdabe84decbcdc) fix(installer): never treat noisy output as "package manager busy" - [`83659e5`](https://github.com/amnezia-vpn/amnezia-client/commit/83659e580deb7270f8d64cabf5382265bc855320) fix(installer): never treat noisy output as "package manager busy" - [`67cd37d`](https://github.com/amnezia-vpn/amnezia-client/commit/67cd37d56ad366fe5185d13b13883cca29538982) fix(installer): keep the package-manager check honest without psmisc or a working sudo - [`58d9ed4`](https://github.com/amnezia-vpn/amnezia-client/commit/58d9ed4bfaf9c057f81e68fb2f573beb04b36f7a) fix(installer): make the lock check immune to symlinked lock paths - [`77dabc9`](https://github.com/amnezia-vpn/amnezia-client/commit/77dabc9b4e4932cffd170fb8b9d77e07ddd072ae) fix(installer): never answer SERVER_FREE with a blind lock scan ### 📊 Changes **2 files changed** (+156 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `client/core/controllers/selfhosted/installController.cpp` (+44 -10) 📝 `client/server_scripts/check_server_is_busy.sh` (+112 -7) </details> ### 📄 Description ### Problem `InstallController::isServerDpkgBusy()` treated *any* non-empty output of `check_server_is_busy.sh` (stdout **and** stderr) as "the package manager is busy" and retried up to 30 times with 10 s sleeps — i.e. a free server could make the app appear to hang for up to 5 minutes in the middle of a server update: ```cpp if (stdOut.isEmpty()) { return ErrorCode::NoError; // free } else { emit serverIsBusy(true); QThread::msleep(10000); // retry, whatever the output was } ``` The script itself was equally fragile: it decided "busy" from a non-empty output of `fuser`/`cat` rather than from their exit status. Reported in #3232 (client on Fedora 43, server on Debian 12: changing Xray settings hangs on the `check_server_is_busy.sh` step). ### Change `client/server_scripts/check_server_is_busy.sh` - decides by exit code (`fuser -s`), wrapped in `timeout 15` when available, so a stuck check cannot block the ssh channel; - prints the explicit marker `SERVER_BUSY` **only** when the lock is really held, and stays silent otherwise; - keeps the legacy strings (`Packet manager not found`, `fuser not installed`, `cat not installed`) for backward compatibility; - uses `command -v` instead of `which || command -v` duplication. `client/core/controllers/selfhosted/installController.cpp` - busy is reported only when the output contains `SERVER_BUSY`; - otherwise the result of `runScript()` is propagated, so an ssh error is no longer silently treated as "server is free"; - the 10 s retry sleep is split into 100 ms steps, so cancelling the installation is honoured immediately; - stale comment fixed (30 attempts, not 100). ### Limitations (important) - The root cause was **not reproduced locally** — there is no reproduction environment. The change is based on static analysis of the code path and is verified statically only; no build/test run (Qt toolchain is not available in my environment). - An independent read-only review found no blocking issues. Non-blocking notes: if the server is genuinely busy but the marker is not printed, the failure surfaces later in the installation flow instead of being retried here — that trade-off is deliberate. - Related, but out of scope for this PR: other places that can make an ssh step wait indefinitely (`SshSession::runScript`, missing `SSH_OPTIONS_TIMEOUT`, stderr read after stdout). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-14 21:07:50 +00:00

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

amnezia-vpn/amnezia-client#3148

No description provided.

Rows
Columns