fix(cli): wait out slow PackageManager1 activation (closes #1338)#1743
fix(cli): wait out slow PackageManager1 activation (closes #1338)#1743123123213weqw wants to merge 1 commit into
Conversation
ll-cli probed org.deepin.linglong.PackageManager1 with a single Ping() that used the default 25s D-Bus timeout. The package manager daemon is D-Bus activated: before it can acquire its well-known bus name it has to be started by systemd and initialize the local ostree repository. On a cold start (and on slower systems/distros such as Ubuntu Kylin and Arch) this can exceed the single 25s window, so ll-cli surfaced a cryptic "org.freedesktop.DBus.Error.NoReply" and gave up immediately, which is what issue OpenAtom-Linyaps#1338 reports. Poll the peer instead: retry Ping() with a short per-call timeout up to a 90s overall deadline (matching systemd's default Type=dbus unit timeout), so a slow but successful activation is waited out rather than reported as a hard failure. When activation ultimately fails, point the user at the daemon service status and journal for actionable diagnostics. Closes OpenAtom-Linyaps#1338. Log: ll-cli now retries the PackageManager1 readiness check instead of failing on the first D-Bus timeout. Influence: only affects ll-cli's PackageManager1 activation path (waitForDBusPeerReady and the activation error message); no public API or behavior change for an already-running daemon. Signed-off-by: 王越 <1939455790@qq.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 123123213weqw The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @123123213weqw. Thanks for your PR. I'm waiting for a OpenAtom-Linyaps member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a retry loop in waitForDBusPeerReady to wait for a DBus peer to become ready, using a 90-second total deadline, a 5-second timeout per call, and a 1-second retry interval. It also improves the error message when failing to activate the DBus package manager by providing systemd troubleshooting commands. The review feedback suggests optimizing the retry loop by instantiating the DBusPeer object and setting its timeout once outside the loop instead of recreating it on every iteration.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| using namespace std::chrono_literals; | ||
|
|
||
| return LINGLONG_OK; | ||
| constexpr auto perCallTimeout = 5s; | ||
| constexpr auto retryInterval = 1s; | ||
| constexpr auto totalDeadline = 90s; | ||
|
|
||
| const auto startedAt = std::chrono::steady_clock::now(); | ||
| while (true) { | ||
| auto peer = linglong::api::dbus::v1::DBusPeer(service, path, connection); | ||
| peer.setTimeout(static_cast<int>(std::chrono::milliseconds(perCallTimeout).count())); | ||
|
|
||
| auto reply = peer.Ping(); |
There was a problem hiding this comment.
Recreating the DBusPeer object and setting its timeout on every single iteration of the loop is inefficient and unnecessary. We can instantiate peer and set its timeout once outside the loop. Additionally, defining perCallTimeout directly in milliseconds (5000ms) simplifies the setTimeout call by avoiding the need for explicit duration conversion.
using namespace std::chrono_literals;
constexpr auto perCallTimeout = 5000ms;
constexpr auto retryInterval = 1s;
constexpr auto totalDeadline = 90s;
auto peer = linglong::api::dbus::v1::DBusPeer(service, path, connection);
peer.setTimeout(static_cast<int>(perCallTimeout.count()));
const auto startedAt = std::chrono::steady_clock::now();
while (true) {
auto reply = peer.Ping();
Codecov Report❌ Patch coverage is
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
2 participants
Summary
ll-clifailed withFailed to activate org.deepin.linglong.PackageManager1 QDBusError("org.freedesktop.DBus.Error.NoReply", ...)on systems such as Ubuntu Kylin 25.04 and Arch Linux (#1338), preventing installs from working even though the package itself was installed.Root cause
Cli::initializeDBusPackageManager()waits for the package manager daemon to be reachable by issuing a singleorg.freedesktop.DBus.Peer.Ping()(libs/linglong/src/linglong/cli/cli.cpp,waitForDBusPeerReady) using the default D-Bus call timeout of 25s.The daemon (
org.deepin.linglong.PackageManager1) is D-Bus activated: the firstPing()asks systemd to startorg.deepin.linglong.PackageManager.service(Type=dbus), and the daemon only acquires its well-known bus name after starting up and initializing the local ostree repository. On a cold start (and on slower systems/distros), this whole sequence can take longer than the single 25s window, so the one-shotPing()times out andll-clireports a hard, crypticNoReplyfailure and aborts.Fix
waitForDBusPeerReadynow polls instead of giving up on the first timeout:Ping()with a short per-call timeout (5s) up to an overall deadline of 90s, which matches systemd's defaultType=dbusunit timeout — so a slow but successful activation is waited out rather than treated as a failure;The
initializeDBusPackageManager()error message now also points the user at the daemon service status and journal, so a genuinely broken daemon can be diagnosed:The change applies to both activation paths (system bus and
--no-dbuspeer mode) since both go throughwaitForDBusPeerReady.Files changed
libs/linglong/src/linglong/cli/cli.cppwaitForDBusPeerReady: bounded retry loop with a per-call timeout and a 90s total deadline.Cli::initializeDBusPackageManager: clearer, actionable activation error message.Verification
Built from source on Debian/trixie with the project's
debugCMake preset (Qt5, ostree, systemd, etc.):cmake --preset debug -G Ninjaconfigured cleanly.linglonglib,ll-cli,ll-package-manager, andll-tests— all link successfully; no new warnings in the edited regions (remaining-Wmissing-field-initializerswarnings are pre-existing in unrelated functions).clang-format(repo.clang-format) produces zero changes on the edited file.ll-tests --gtest_filter='Cli*:*RepoAndPackageManager*'→ 20/20 PASSED.Error.WarpStdErrorCode) and theLayerPackagerTest/UabFileTestsuite-setup failures are pre-existing and environmental: they reproduce identically on the unmodified tree (verified viagit stash/rebuild) and are caused by running as root and missing FUSE tooling in this container, not by this change.Notes / assumptions
Type=dbusTimeoutSecso the client waits at least as long as the daemon's supervisor does.waitForDBusPeerReadyis a file-local helper, so the retry logic is covered indirectly by the existingCliRepoAndPackageManager*tests (which exercise the surroundinggetPkgMan/getRepoflow). A direct unit test would require mocking aQDBusConnection, which is out of scope for this minimal fix.Closes #1338.