feat: avoid duplicate ll-package-manager startup by checking existing#1707
Conversation
connection 1. Add `startProcess` helper to launch detached processes with proper environment and cleanup on exit. 2. Replace unconditional `startDetached` and sleep with a D-Bus peer connection check to see if `ll-package-manager` is already running. 3. Only start a new process when no existing daemon is detected; otherwise log reuse of existing instance. 4. Include signal handling to terminate spawned processes when the application quits. Log: Improved process management for ll-package-manager to prevent duplicate instances. Influence: 1. Verify that ll-package-manager starts correctly when not already running. 2. Test that when ll-package-manager is already running, a new process is not spawned. 3. Confirm that spawned processes are terminated when the main application exits. 4. Check logging output for "reuse existing process" or "start" messages. 5. Verify D-Bus connection to the peer socket works as expected. 6. Test in scenarios where the socket file might be stale or missing.
![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 optimizes the startup of ll-package-manager by checking if it is already running via a peer-to-peer DBus connection before attempting to spawn a new process. It also introduces a helper function startProcess to handle starting the daemon detached and registering a cleanup hook to terminate it upon application exit. The review feedback highlights two critical issues: first, if startDetached fails, the PID remains 0, causing kill(0, SIGTERM) to prematurely terminate the entire process group; second, a failed connectToPeer call leaves a stale, unconnected connection in the global registry, which must be explicitly cleaned up using disconnectFromPeer to avoid caching issues in subsequent connection attempts.
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.
| qint64 pid = 0; | ||
| process.startDetached(&pid); | ||
|
|
||
| LogD("start {} {} as {}", program.toStdString(), args.join(" ").toStdString(), pid); | ||
|
|
||
| QObject::connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, [pid]() { | ||
| LogD("kill {}", pid); | ||
| kill(pid, SIGTERM); | ||
| }); |
There was a problem hiding this comment.
If QProcess::startDetached fails to start the process, pid will remain 0. Calling kill(0, SIGTERM) inside the aboutToQuit lambda will send SIGTERM to all processes in the current process group, terminating the calling application (ll-cli) and any other sibling processes prematurely.
We must check the return value of startDetached and ensure pid > 0 before registering the cleanup connection.
qint64 pid = 0;
if (!process.startDetached(&pid) || pid <= 0) {
LogE("failed to start process: {}", program.toStdString());
return;
}
LogD("start {} {} as {}", program.toStdString(), args.join(" ").toStdString(), pid);
QObject::connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, [pid]() {
LogD("kill {}", pid);
kill(pid, SIGTERM);
});| // Check if ll-package-manager is already running | ||
| auto pkgManConn = QDBusConnection::connectToPeer(pkgManAddress, "ll-package-manager"); | ||
| if (!pkgManConn.isConnected()) { | ||
| // Only start a new process if no existing one is listening | ||
| startProcess("sudo", | ||
| { "--user", | ||
| LINGLONG_USERNAME, | ||
| "--preserve-env=QT_FORCE_STDERR_LOGGING", | ||
| "--preserve-env=QDBUS_DEBUG", | ||
| LINGLONG_LIBEXEC_DIR "/ll-package-manager", | ||
| "--no-dbus" }); | ||
| using namespace std::chrono_literals; | ||
| std::this_thread::sleep_for(1s); | ||
| } else { | ||
| LogD("ll-package-manager is already running, reuse existing process"); | ||
| } |
There was a problem hiding this comment.
When QDBusConnection::connectToPeer is called, it registers the connection globally under the specified name ("ll-package-manager"). If the connection attempt fails, this unconnected connection remains registered in the global registry.
Any subsequent call to QDBusConnection::connectToPeer with the same name (such as in Cli::getPkgMan()) will return this cached, unconnected connection instead of attempting a new connection to the newly spawned daemon.
To prevent this, we must call QDBusConnection::disconnectFromPeer("ll-package-manager") if isConnected() returns false.
// Check if ll-package-manager is already running
auto pkgManConn = QDBusConnection::connectToPeer(pkgManAddress, "ll-package-manager");
if (!pkgManConn.isConnected()) {
QDBusConnection::disconnectFromPeer("ll-package-manager");
// Only start a new process if no existing one is listening
startProcess("sudo",
{ "--user",
LINGLONG_USERNAME,
"--preserve-env=QT_FORCE_STDERR_LOGGING",
"--preserve-env=QDBUS_DEBUG",
LINGLONG_LIBEXEC_DIR "/ll-package-manager",
"--no-dbus" });
using namespace std::chrono_literals;
std::this_thread::sleep_for(1s);
} else {
LogD("ll-package-manager is already running, reuse existing process");
}
deepin pr auto review☀
|
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
|
@dengbo11: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dengbo11, reddevillg 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 |
3 participants
connection
startProcesshelper to launch detached processes with proper environment and cleanup on exit.startDetachedand sleep with a D-Bus peer connection check to see ifll-package-manageris already running.Log: Improved process management for ll-package-manager to prevent duplicate instances.
Influence: