From f7e6cc986c38cf07dd3eb382ac90f7ec56195048 Mon Sep 17 00:00:00 2001 From: reddevillg Date: Mon, 8 Jun 2026 13:15:45 +0800 Subject: [PATCH] fix: fix regression by 9937c5456e16dd1dd764b946ed382c0095048c53 ReplyInteraction signal may lost if Task object is not ready. Move it from per-task interface to the PackageManager1 interface. Add syncTaskProperties to fetch initial task state also. Signed-off-by: reddevillg --- .../org.deepin.linglong.PackageManager1.xml | 11 ++ api/dbus/org.deepin.linglong.Task1.xml | 9 -- libs/linglong/src/linglong/cli/cli.cpp | 70 ++++++++--- libs/linglong/src/linglong/cli/cli.h | 5 +- .../package_manager/package_manager.cpp | 116 +++++++++++++++++- .../package_manager/package_manager.h | 16 +++ .../linglong/package_manager/package_task.cpp | 101 --------------- .../linglong/package_manager/package_task.h | 16 +-- .../package_manager/ref_installation.cpp | 5 +- .../package_manager/uab_installation.cpp | 4 +- 10 files changed, 207 insertions(+), 146 deletions(-) diff --git a/api/dbus/org.deepin.linglong.PackageManager1.xml b/api/dbus/org.deepin.linglong.PackageManager1.xml index d2f2fb15f..a11f69708 100644 --- a/api/dbus/org.deepin.linglong.PackageManager1.xml +++ b/api/dbus/org.deepin.linglong.PackageManager1.xml @@ -67,6 +67,11 @@ SPDX-License-Identifier: LGPL-3.0-or-later + + + + + @@ -76,5 +81,11 @@ SPDX-License-Identifier: LGPL-3.0-or-later + + + + + + diff --git a/api/dbus/org.deepin.linglong.Task1.xml b/api/dbus/org.deepin.linglong.Task1.xml index debd882ec..ea3584745 100644 --- a/api/dbus/org.deepin.linglong.Task1.xml +++ b/api/dbus/org.deepin.linglong.Task1.xml @@ -7,18 +7,9 @@ SPDX-License-Identifier: LGPL-3.0-or-later - - - - - - - - - diff --git a/libs/linglong/src/linglong/cli/cli.cpp b/libs/linglong/src/linglong/cli/cli.cpp index 69a095fb0..24e630b68 100644 --- a/libs/linglong/src/linglong/cli/cli.cpp +++ b/libs/linglong/src/linglong/cli/cli.cpp @@ -51,6 +51,8 @@ #include #include +#include +#include #include #include #include @@ -293,9 +295,14 @@ void Cli::onTaskPropertiesChanged( handleTaskState(); } -void Cli::onTaskRequestInteraction(int messageID, const QVariantMap &additionalMessage) +void Cli::interaction(const QDBusObjectPath &object_path, + int messageID, + const QVariantMap &additionalMessage) { LINGLONG_TRACE("interactive with user") + if (object_path.path() != taskObjectPath) { + return; + } auto messageType = static_cast(messageID); auto msg = common::serialize::fromQVariantMap< @@ -346,13 +353,14 @@ void Cli::onTaskRequestInteraction(int messageID, const QVariantMap &additionalM LogD("action: {}", action); auto reply = api::types::v1::InteractionReply{ .action = action }; - if (!this->task) { - this->printer.printErr(LINGLONG_ERRV("task object is null")); + auto pkgMan = this->getPkgMan(); + if (!pkgMan) { + this->printer.printErr(pkgMan.error()); return; } QDBusPendingReply dbusReply = - this->task->ReplyInteraction(common::serialize::toQVariantMap(reply)); + (*pkgMan)->ReplyInteraction(object_path, common::serialize::toQVariantMap(reply)); dbusReply.waitForFinished(); if (dbusReply.isError()) { this->printer.printErr( @@ -535,7 +543,7 @@ utils::error::Result Cli::initPkgManSignals() if (!conn.connect(pkgMan->service(), pkgMan->path(), pkgMan->interface(), - "TaskAdd", + "TaskAdded", this, SLOT(onTaskAdded(QDBusObjectPath)))) { return LINGLONG_ERR("couldn't connect to package manager signal 'TaskAdded'"); @@ -550,6 +558,16 @@ utils::error::Result Cli::initPkgManSignals() return LINGLONG_ERR("couldn't connect to package manager signal 'TaskRemoved'"); } + if (!conn.connect(pkgMan->service(), + pkgMan->path(), + pkgMan->interface(), + "RequestInteraction", + this, + SLOT(interaction(QDBusObjectPath, int, QVariantMap)))) { + return LINGLONG_ERR(fmt::format("Failed to connect signal RequestInteraction: {}", + conn.lastError().message().toStdString())); + } + this->pkgManSignalsInitialized = true; return LINGLONG_OK; } @@ -2246,6 +2264,30 @@ int Cli::getBundleDir(const InspectOptions &options) return 0; } +utils::error::Result Cli::syncTaskProperties() +{ + LINGLONG_TRACE("syncTaskProperties"); + + auto pkgMan = this->getPkgMan(); + if (!pkgMan) { + return LINGLONG_ERR(pkgMan); + } + + QDBusInterface properties((*pkgMan)->service(), + this->taskObjectPath, + "org.freedesktop.DBus.Properties", + (*pkgMan)->connection()); + QDBusReply reply = + properties.call("GetAll", QStringLiteral("org.deepin.linglong.Task1")); + if (!reply.isValid()) { + return LINGLONG_ERR( + fmt::format("failed to get task properties: {}", reply.error().message().toStdString())); + } + + this->onTaskPropertiesChanged(QStringLiteral("org.deepin.linglong.Task1"), reply.value(), {}); + return LINGLONG_OK; +} + utils::error::Result Cli::waitTaskCreated(QDBusPendingReply &reply, TaskType taskType) { @@ -2285,21 +2327,17 @@ utils::error::Result Cli::waitTaskCreated(QDBusPendingReply & conn.lastError().message().toStdString())); } - if (!conn.connect((*pkgMan)->service(), - taskObjectPath, - "org.deepin.linglong.Task1", - "RequestInteraction", - this, - SLOT(onTaskRequestInteraction(int, QVariantMap)))) { - return LINGLONG_ERR(fmt::format("Failed to connect signal RequestInteraction: {}", - conn.lastError().message().toStdString())); - } - - return LINGLONG_OK; + return this->syncTaskProperties(); } void Cli::waitTaskDone() { + if (this->taskState.state == api::types::v1::State::Failed + || this->taskState.state == api::types::v1::State::Canceled + || this->taskState.state == api::types::v1::State::Succeed) { + return; + } + QEventLoop loop; if (QObject::connect(this, &Cli::taskDone, &loop, &QEventLoop::quit) == nullptr) { LogE("connect taskDone failed"); diff --git a/libs/linglong/src/linglong/cli/cli.h b/libs/linglong/src/linglong/cli/cli.h index 24c598c4b..fe869de76 100644 --- a/libs/linglong/src/linglong/cli/cli.h +++ b/libs/linglong/src/linglong/cli/cli.h @@ -229,6 +229,7 @@ class Cli : public QObject void detectDrivers(); utils::error::Result getPkgMan(); utils::error::Result initPkgManSignals(); + utils::error::Result syncTaskProperties(); int runResolvedContext(runtime::RunContext &runContext, const RunOptions &options, std::optional runtimeConfig); @@ -272,7 +273,9 @@ private Q_SLOTS: void onTaskPropertiesChanged(const QString &interface, const QVariantMap &changed_properties, const QStringList &invalidated_properties); - void onTaskRequestInteraction(int messageID, const QVariantMap &additionalMessage); + void interaction(const QDBusObjectPath &object_path, + int messageID, + const QVariantMap &additionalMessage); Q_SIGNALS: void taskDone(); diff --git a/libs/linglong/src/linglong/package_manager/package_manager.cpp b/libs/linglong/src/linglong/package_manager/package_manager.cpp index 95a721ce3..c76a6870a 100644 --- a/libs/linglong/src/linglong/package_manager/package_manager.cpp +++ b/libs/linglong/src/linglong/package_manager/package_manager.cpp @@ -9,6 +9,7 @@ #include "configure.h" #include "linglong/api/types/helper.h" #include "linglong/api/types/v1/Generators.hpp" // IWYU pragma: keep +#include "linglong/api/types/v1/InteractionReply.hpp" #include "linglong/api/types/v1/PackageInfoV2.hpp" #include "linglong/api/types/v1/PackageManager1JobInfo.hpp" #include "linglong/api/types/v1/PackageManager1PruneResult.hpp" @@ -45,12 +46,14 @@ #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -655,7 +658,7 @@ QVariantMap PackageManager::installFromLayer(const QDBusUnixFileDescriptor &fd, PackageTask &taskRef = dynamic_cast(task); if (msgType == api::types::v1::InteractionMessageType::Upgrade && !options.skipInteraction) { - if (!taskRef.waitConfirm(msgType, additionalMessage)) { + if (!this->waitConfirm(taskRef, msgType, additionalMessage)) { return; } } @@ -1887,6 +1890,117 @@ auto PackageManager::InitRunContext(const QString &runContextCfg, }); } +void PackageManager::ReplyInteraction(QDBusObjectPath object_path, const QVariantMap &replies) +{ + Q_EMIT this->ReplyReceived(object_path, replies); +} + +void PackageManager::onPeerDisconnected() noexcept +{ + LogW("peer caller disconnected"); + Q_EMIT CallerDisconnected(); +} + +bool PackageManager::waitConfirm( + PackageTask &taskRef, + api::types::v1::InteractionMessageType msgType, + const api::types::v1::PackageManager1RequestInteractionAdditionalMessage + &additionalMessage) noexcept +{ + auto objectPath = QDBusObjectPath(taskRef.taskObjectPath().c_str()); + QEventLoop loop; + QTimer timeoutTimer; + timeoutTimer.setSingleShot(true); + + auto callerContext = taskRef.callerContext(); + std::unique_ptr watcher; + + QMetaObject::Connection callerDisconnectedConn; + + if (callerContext.isPeerMode()) { + callerDisconnectedConn = + connect(this, &PackageManager::CallerDisconnected, &loop, [&taskRef, &loop]() { + taskRef.updateState(linglong::api::types::v1::State::Canceled, "caller disconnected"); + loop.exit(1); + }); + + auto connected = callerContext.connection.connect("", + "/org/freedesktop/DBus/Local", + "org.freedesktop.DBus.Local", + "Disconnected", + this, + SLOT(onPeerDisconnected())); + if (!connected) { + LogW("failed to connect to Disconnected signal for peer mode"); + } + } else { + auto callerName = callerContext.callerBusName(); + if (!callerName.isEmpty()) { + watcher = + std::make_unique(callerName, + callerContext.connection, + QDBusServiceWatcher::WatchForUnregistration); + connect(watcher.get(), + &QDBusServiceWatcher::serviceUnregistered, + &loop, + [&taskRef, &loop, callerName]() { + LogW("caller {} disconnected", callerName.toStdString()); + taskRef.updateState(linglong::api::types::v1::State::Canceled, + "caller disconnected"); + loop.exit(1); + }); + } + } + + auto replyConn = + connect(this, + &PackageManager::ReplyReceived, + &loop, + [&taskRef, &loop, objectPath](const QDBusObjectPath &replyObjectPath, + const QVariantMap &reply) { + if (replyObjectPath.path() != objectPath.path()) { + return; + } + + auto interactionReply = + common::serialize::fromQVariantMap(reply); + if (!interactionReply || interactionReply->action != "yes") { + taskRef.updateState(linglong::api::types::v1::State::Canceled, "canceled"); + } + loop.exit(0); + }); + + auto timeoutConn = connect(&timeoutTimer, &QTimer::timeout, &loop, [&taskRef, &loop]() { + auto msg = fmt::format("task {} confirm timeout", taskRef.taskID()); + LogW(msg); + taskRef.updateState(linglong::api::types::v1::State::Canceled, msg); + loop.exit(1); + }); + + Q_EMIT RequestInteraction(objectPath, + static_cast(msgType), + common::serialize::toQVariantMap(additionalMessage)); + + timeoutTimer.start(std::chrono::milliseconds{ 180000 }); + loop.exec(); + timeoutTimer.stop(); + + disconnect(replyConn); + disconnect(timeoutConn); + disconnect(callerDisconnectedConn); + + if (callerContext.isPeerMode()) { + callerContext.connection.disconnect("", + "/org/freedesktop/DBus/Local", + "org.freedesktop.DBus.Local", + "Disconnected", + this, + SLOT(onPeerDisconnected())); + } + + return !taskRef.isTaskDone(); +} + // no-op for now utils::error::Result PackageManager::tryGenerateCache(const package::Reference &ref) noexcept { diff --git a/libs/linglong/src/linglong/package_manager/package_manager.h b/libs/linglong/src/linglong/package_manager/package_manager.h index 30ad2fac7..17bf28223 100644 --- a/libs/linglong/src/linglong/package_manager/package_manager.h +++ b/libs/linglong/src/linglong/package_manager/package_manager.h @@ -8,6 +8,8 @@ #include "linglong/api/types/v1/CommonOptions.hpp" #include "linglong/api/types/v1/ContainerProcessStateInfo.hpp" +#include "linglong/api/types/v1/InteractionMessageType.hpp" +#include "linglong/api/types/v1/PackageManager1RequestInteractionAdditionalMessage.hpp" #include "linglong/api/types/v1/Repo.hpp" #include "linglong/api/types/v1/UabLayer.hpp" #include "linglong/package/fuzzy_reference.h" @@ -20,6 +22,7 @@ #include #include #include +#include #include #include @@ -58,6 +61,7 @@ public auto Update(const QVariantMap ¶meters) noexcept -> QVariantMap; auto Search(const QVariantMap ¶meters) noexcept -> QVariantMap; auto Prune() noexcept -> QVariantMap; + void ReplyInteraction(QDBusObjectPath object_path, const QVariantMap &replies); auto InitRunContext(const QString &runContextCfg, const QString &containerID) noexcept -> QVariantMap; @@ -73,6 +77,10 @@ public virtual utils::error::Result tryGenerateCache(const package::Reference &ref) noexcept; utils::error::Result executePostInstallHooks(const package::Reference &ref) noexcept; utils::error::Result executePostUninstallHooks(const package::Reference &ref) noexcept; + bool waitConfirm(PackageTask &taskRef, + api::types::v1::InteractionMessageType msgType, + const api::types::v1::PackageManager1RequestInteractionAdditionalMessage + &additionalMessage) noexcept; virtual utils::error::Result installAppDepends(Task &task, const api::types::v1::PackageInfoV2 &app); @@ -107,9 +115,17 @@ public Q_SIGNALS: void TaskAdded(QDBusObjectPath object_path); void TaskRemoved(QDBusObjectPath object_path); + void RequestInteraction(QDBusObjectPath object_path, + int messageID, + QVariantMap additionalMessage); + void CallerDisconnected(); void SearchFinished(QString jobID, QVariantMap result); void PruneFinished(QString jobID, QVariantMap result); void InitRunContextFinished(QString jobID, bool success); + void ReplyReceived(QDBusObjectPath object_path, const QVariantMap &replies); + +private Q_SLOTS: + void onPeerDisconnected() noexcept; private: QVariantMap installFromLayer(const QDBusUnixFileDescriptor &fd, diff --git a/libs/linglong/src/linglong/package_manager/package_task.cpp b/libs/linglong/src/linglong/package_manager/package_task.cpp index 0f6ace03c..7e6c3a479 100644 --- a/libs/linglong/src/linglong/package_manager/package_task.cpp +++ b/libs/linglong/src/linglong/package_manager/package_task.cpp @@ -6,7 +6,6 @@ #include "linglong/adaptors/task/task1.h" #include "linglong/common/dbus/register.h" -#include "linglong/common/serialize/json.h" #include "linglong/package_manager/package_manager.h" #include "linglong/utils/error/error.h" #include "linglong/utils/log/formatter.h" // IWYU pragma: keep @@ -14,9 +13,6 @@ #include #include -#include -#include - #include namespace linglong::service { @@ -89,108 +85,11 @@ void PackageTask::Cancel() noexcept g_cancellable_cancel(m_cancelFlag); } -void PackageTask::ReplyInteraction(const QVariantMap &replies) noexcept -{ - Q_EMIT this->ReplyReceived(replies); -} - -void PackageTask::onPeerDisconnected() noexcept -{ - LogW("peer caller disconnected"); - Q_EMIT peerDisconnected(); -} - void PackageTask::setCallerContext(const CallerContext &ctx) { m_callerContext = ctx; } -bool PackageTask::waitConfirm( - api::types::v1::InteractionMessageType msgType, - const api::types::v1::PackageManager1RequestInteractionAdditionalMessage &additionalMessage, - std::chrono::milliseconds timeout) noexcept -{ - Q_EMIT RequestInteraction(static_cast(msgType), - common::serialize::toQVariantMap(additionalMessage)); - QEventLoop loop; - - QTimer timeoutTimer; - timeoutTimer.setSingleShot(true); - - std::unique_ptr watcher; - - auto peerDisconnectedConn = - QObject::connect(this, &PackageTask::peerDisconnected, &loop, [&loop, this]() { - updateState(linglong::api::types::v1::State::Canceled, "caller disconnected"); - loop.exit(1); - }); - - if (m_callerContext.isPeerMode()) { - auto connected = m_callerContext.connection.connect("", - "/org/freedesktop/DBus/Local", - "org.freedesktop.DBus.Local", - "Disconnected", - this, - SLOT(onPeerDisconnected())); - if (!connected) { - LogW("failed to connect to Disconnected signal for peer mode"); - } - } else { - auto callerName = m_callerContext.callerBusName(); - if (!callerName.isEmpty()) { - watcher = - std::make_unique(callerName, - m_callerContext.connection, - QDBusServiceWatcher::WatchForUnregistration); - connect( - watcher.get(), - &QDBusServiceWatcher::serviceUnregistered, - &loop, - [&loop, this]() { - LogW("caller {} disconnected", m_callerContext.callerBusName().toStdString()); - updateState(linglong::api::types::v1::State::Canceled, "caller disconnected"); - loop.exit(1); - }); - } - } - - auto replyConn = - connect(this, &PackageTask::ReplyReceived, &loop, [this, &loop](const QVariantMap &reply) { - auto interactionReply = - common::serialize::fromQVariantMap(reply); - if (!interactionReply || interactionReply->action != "yes") { - updateState(linglong::api::types::v1::State::Canceled, "canceled"); - } - loop.exit(0); - }); - - auto timeoutConn = connect(&timeoutTimer, &QTimer::timeout, &loop, [this, &loop]() { - auto msg = fmt::format("task {} confirm timeout", taskID()); - LogW(msg); - updateState(linglong::api::types::v1::State::Canceled, msg); - loop.exit(1); - }); - - timeoutTimer.start(timeout); - loop.exec(); - timeoutTimer.stop(); - - disconnect(replyConn); - disconnect(timeoutConn); - disconnect(peerDisconnectedConn); - - if (m_callerContext.isPeerMode()) { - m_callerContext.connection.disconnect("", - "/org/freedesktop/DBus/Local", - "org.freedesktop.DBus.Local", - "Disconnected", - this, - SLOT(onPeerDisconnected())); - } - - return !isTaskDone(); -} - utils::error::Result PackageTask::exposeOnDBus() noexcept { LINGLONG_TRACE(fmt::format("expose task {} on dbus", taskID())); diff --git a/libs/linglong/src/linglong/package_manager/package_task.h b/libs/linglong/src/linglong/package_manager/package_task.h index b34dff335..0b7faf0d2 100644 --- a/libs/linglong/src/linglong/package_manager/package_task.h +++ b/libs/linglong/src/linglong/package_manager/package_task.h @@ -4,8 +4,6 @@ #pragma once -#include "linglong/api/types/v1/InteractionMessageType.hpp" -#include "linglong/api/types/v1/PackageManager1RequestInteractionAdditionalMessage.hpp" #include "linglong/api/types/v1/State.hpp" #include "linglong/common/dbus/properties_forwarder.h" #include "linglong/package_manager/task.h" @@ -22,7 +20,6 @@ #include #include -#include #include #include #include @@ -32,8 +29,6 @@ Q_DECLARE_METATYPE(linglong::api::types::v1::State) namespace linglong::service { -using namespace std::chrono_literals; - struct CallerContext { QDBusConnection connection{ QDBusConnection::systemBus() }; @@ -93,15 +88,10 @@ class PackageTask : public QObject, protected QDBusContext, public Task, public void setCallerContext(const CallerContext &ctx); - bool waitConfirm( - api::types::v1::InteractionMessageType msgType, - const api::types::v1::PackageManager1RequestInteractionAdditionalMessage &additionalMessage, - std::chrono::milliseconds timeout = 180000ms) noexcept; + [[nodiscard]] const CallerContext &callerContext() const noexcept { return m_callerContext; } public Q_SLOTS: void Cancel() noexcept; - void ReplyInteraction(const QVariantMap &replies) noexcept; - void onPeerDisconnected() noexcept; Q_SIGNALS: void StateChanged(int newState); @@ -113,10 +103,6 @@ public Q_SLOTS: void changePropertiesDone(); - void RequestInteraction(int messageID, QVariantMap additionalMessage); - void ReplyReceived(const QVariantMap &replies); - void peerDisconnected(); - private: friend class PackageTaskQueue; GCancellable *m_cancelFlag{ nullptr }; diff --git a/libs/linglong/src/linglong/package_manager/ref_installation.cpp b/libs/linglong/src/linglong/package_manager/ref_installation.cpp index 18d4473f2..be4923d6c 100644 --- a/libs/linglong/src/linglong/package_manager/ref_installation.cpp +++ b/libs/linglong/src/linglong/package_manager/ref_installation.cpp @@ -173,8 +173,9 @@ utils::error::Result RefInstallationAction::preInstall(Task &task) .localRef = operation->oldRef->toString(), .remoteRef = operation->newRef->reference.toString() }; - if (!mainTask->waitConfirm(api::types::v1::InteractionMessageType::Upgrade, - additionalMessage)) { + if (!pm.waitConfirm(*mainTask, + api::types::v1::InteractionMessageType::Upgrade, + additionalMessage)) { return LINGLONG_ERR("action canceled"); } } diff --git a/libs/linglong/src/linglong/package_manager/uab_installation.cpp b/libs/linglong/src/linglong/package_manager/uab_installation.cpp index 53ca8cf00..a56f0bf29 100644 --- a/libs/linglong/src/linglong/package_manager/uab_installation.cpp +++ b/libs/linglong/src/linglong/package_manager/uab_installation.cpp @@ -288,7 +288,9 @@ utils::error::Result UabInstallationAction::preInstall(PackageTask &task) .localRef = operation->oldRef->toString(), .remoteRef = operation->newRef->reference.toString() }; - if (!task.waitConfirm(api::types::v1::InteractionMessageType::Upgrade, additionalMessage)) { + if (!pm.waitConfirm(task, + api::types::v1::InteractionMessageType::Upgrade, + additionalMessage)) { return LINGLONG_ERR("action canceled"); } }