From db46ad25860524734049aee2d209c5697802ea9a Mon Sep 17 00:00:00 2001 From: yangxuguang-huawei Date: Fri, 14 Aug 2026 16:01:56 +0800 Subject: [PATCH] bugfix: agent cppcrash Signed-off-by: yangxuguang-huawei AI[0%] Human Fixed[0%] Human[100%] AI Adopted[0%] Change-Id: I00a8035c811778529027d9518f071e1d689fa871 --- .../include/connection/js_agent_connection.h | 16 +++--- .../src/connection/js_agent_connection.cpp | 50 ++++++++++++------- .../agent_manager/src/js_agent_manager.cpp | 30 +++++++---- 3 files changed, 61 insertions(+), 35 deletions(-) diff --git a/agent_runtime_framework/frameworks/js/napi/agent_extension_ability/include/connection/js_agent_connection.h b/agent_runtime_framework/frameworks/js/napi/agent_extension_ability/include/connection/js_agent_connection.h index e37f104cdd..6735563501 100644 --- a/agent_runtime_framework/frameworks/js/napi/agent_extension_ability/include/connection/js_agent_connection.h +++ b/agent_runtime_framework/frameworks/js/napi/agent_extension_ability/include/connection/js_agent_connection.h @@ -187,7 +187,11 @@ public: */ void SetNapiAsyncTask(std::shared_ptr &task); - void SetDisconnectAsyncTask(const std::shared_ptr &task); + // Returns false without overwriting if a pending task exists (prevents re-entrant orphaning). + bool SetDisconnectAsyncTask(const std::shared_ptr &task); + + // Take ownership; first settler wins. Both guarded by stateLock_. + std::shared_ptr TakeDisconnectAsyncTask(); /** * Add a duplicated pending task. @@ -211,8 +215,9 @@ public: // Reject primary/duplicated/staged low-code tasks, clear primary async task, remove connection. // Public: DoConnectAgentExtensionAbility (js_agent_manager.cpp) rejects staged reuse on AgentManagerService - // connect failure. - void RejectConnectAndCleanup(napi_env env, napi_value error, bool hasPrimaryTask); + // connect failure. Idempotent: takes ownership of napiAsyncTask_ so the first settler wins; callers must not + // pass a stale snapshot flag. + void RejectConnectAndCleanup(napi_env env, napi_value error); void AdoptDuplicatedPendingTasks(std::vector> &&tasks); @@ -373,10 +378,9 @@ private: void HandleOnAbilityDisconnectDone(const AppExecFwk::ElementName &element, int resultCode); // Returns true when the connect flow must abort (no pending task, or result failure). - bool AbortOnConnectError(napi_env env, int resultCode, bool hasPrimaryTask, bool hasDuplicatedPendingTask); + bool AbortOnConnectError(napi_env env, int resultCode, bool hasDuplicatedPendingTask); // Build the JS receiver proxy for the just-connected host; nullptr (after cleanup) on failure. - napi_value BuildAgentReceiverProxy(napi_env env, const sptr &remoteObject, - bool hasPrimaryTask); + napi_value BuildAgentReceiverProxy(napi_env env, const sptr &remoteObject); protected: napi_env env_; diff --git a/agent_runtime_framework/frameworks/js/napi/agent_extension_ability/src/connection/js_agent_connection.cpp b/agent_runtime_framework/frameworks/js/napi/agent_extension_ability/src/connection/js_agent_connection.cpp index ecc5703218..40904f85cb 100644 --- a/agent_runtime_framework/frameworks/js/napi/agent_extension_ability/src/connection/js_agent_connection.cpp +++ b/agent_runtime_framework/frameworks/js/napi/agent_extension_ability/src/connection/js_agent_connection.cpp @@ -268,22 +268,23 @@ void JSAgentConnection::OnAbilityConnectDone(const AppExecFwk::ElementName &elem } // Reject primary + duplicated + staged low-code tasks with error; remove connection from registry. -void JSAgentConnection::RejectConnectAndCleanup(napi_env env, napi_value error, bool hasPrimaryTask) +void JSAgentConnection::RejectConnectAndCleanup(napi_env env, napi_value error) { - if (hasPrimaryTask) { - napiAsyncTask_->Reject(env, error); + // Take ownership so the first settler wins. No lock: all settlers are JS-thread-serialized. + // Do NOT hold stateLock_ — sub-calls re-acquire it (non-recursive mutex). + auto primary = std::move(napiAsyncTask_); + if (primary != nullptr) { + primary->Reject(env, error); } RejectDuplicatedPendingTask(env, error); RejectPendingLowCodeReuseTasks(env, error); - napiAsyncTask_ = nullptr; AgentConnectionUtils::RemoveAgentConnection(connectionId_); } // True when connect must abort: no pending task, or result-code failure (after reject+cleanup). -bool JSAgentConnection::AbortOnConnectError(napi_env env, int resultCode, bool hasPrimaryTask, - bool hasDuplicatedPendingTask) +bool JSAgentConnection::AbortOnConnectError(napi_env env, int resultCode, bool hasDuplicatedPendingTask) { - if (!hasPrimaryTask && !hasDuplicatedPendingTask) { + if (napiAsyncTask_ == nullptr && !hasDuplicatedPendingTask) { TAG_LOGD(AAFwkTag::SER_ROUTER, "No pending connect task"); return true; } @@ -292,13 +293,13 @@ bool JSAgentConnection::AbortOnConnectError(napi_env env, int resultCode, bool h } napi_value error = CreateJsErrorByNativeErr(env, resultCode, "", AbilityRuntime::GetInnerErrorMsg(AbilityRuntime::AbilityInnerErrorMsg::CONNECT_AGENT_EXTENSION_FAILED)); - RejectConnectAndCleanup(env, error, hasPrimaryTask); + RejectConnectAndCleanup(env, error); return true; } // Build the JS receiver proxy for the connected host; nullptr (after reject+cleanup) on creation failure. napi_value JSAgentConnection::BuildAgentReceiverProxy(napi_env env, - const sptr &remoteObject, bool hasPrimaryTask) + const sptr &remoteObject) { sptr hostStub = GetServiceHostStub(); sptr hostProxy = nullptr; @@ -313,7 +314,7 @@ napi_value JSAgentConnection::BuildAgentReceiverProxy(napi_env env, napi_value error = CreateJsErrorByNativeErr(env, static_cast(AbilityRuntime::AbilityErrorCode::ERROR_CODE_INNER), "", AbilityRuntime::GetInnerErrorMsg(AbilityRuntime::AbilityInnerErrorMsg::OPERATION_FAILED)); - RejectConnectAndCleanup(env, error, hasPrimaryTask); + RejectConnectAndCleanup(env, error); return nullptr; } @@ -321,18 +322,17 @@ void JSAgentConnection::HandleOnAbilityConnectDone(const AppExecFwk::ElementName const sptr &remoteObject, int resultCode) { TAG_LOGI(AAFwkTag::SER_ROUTER, "HandleOnAbilityConnectDone, resultCode: %{public}d", resultCode); - bool hasPrimaryTask = napiAsyncTask_ != nullptr; bool hasDuplicatedPendingTask = !duplicatedPendingTaskList_.empty(); - if (AbortOnConnectError(env_, resultCode, hasPrimaryTask, hasDuplicatedPendingTask)) { + if (AbortOnConnectError(env_, resultCode, hasDuplicatedPendingTask)) { return; } - napi_value proxy = BuildAgentReceiverProxy(env_, remoteObject, hasPrimaryTask); + napi_value proxy = BuildAgentReceiverProxy(env_, remoteObject); if (proxy == nullptr) { return; } SetProxyObject(proxy); - if (hasPrimaryTask) { + if (napiAsyncTask_ != nullptr) { napiAsyncTask_->ResolveWithNoError(env_, proxy); } ResolveDuplicatedPendingTask(env_, proxy); @@ -391,13 +391,14 @@ void JSAgentConnection::HandleOnAbilityDisconnectDone(const AppExecFwk::ElementN if (disconnectCompleteHandler_ != nullptr) { disconnectCompleteHandler_(wptr(this)); } - if (disconnectAsyncTask_ != nullptr) { + // Take ownership: first settler wins, the other finds nullptr. + auto disconnectTask = TakeDisconnectAsyncTask(); + if (disconnectTask != nullptr) { if (resultCode == static_cast(AbilityRuntime::AbilityErrorCode::ERROR_OK)) { - disconnectAsyncTask_->ResolveWithNoError(env_, CreateJsUndefined(env_)); + disconnectTask->ResolveWithNoError(env_, CreateJsUndefined(env_)); } else { - disconnectAsyncTask_->Reject(env_, CreateJsErrorByNativeErr(env_, resultCode)); + disconnectTask->Reject(env_, CreateJsErrorByNativeErr(env_, resultCode)); } - disconnectAsyncTask_ = nullptr; } // release connect @@ -429,9 +430,20 @@ void JSAgentConnection::SetNapiAsyncTask(std::shared_ptr &task) +bool JSAgentConnection::SetDisconnectAsyncTask(const std::shared_ptr &task) { + std::lock_guard lock(stateLock_); + if (disconnectAsyncTask_ != nullptr) { + return false; // re-entrant disconnect would orphan the pending promise + } disconnectAsyncTask_ = task; + return true; +} + +std::shared_ptr JSAgentConnection::TakeDisconnectAsyncTask() +{ + std::lock_guard lock(stateLock_); + return std::move(disconnectAsyncTask_); } void JSAgentConnection::AddDuplicatedPendingTask(std::unique_ptr &task) diff --git a/agent_runtime_framework/frameworks/js/napi/agent_manager/src/js_agent_manager.cpp b/agent_runtime_framework/frameworks/js/napi/agent_manager/src/js_agent_manager.cpp index 88d3bf7c7f..9286cce235 100644 --- a/agent_runtime_framework/frameworks/js/napi/agent_manager/src/js_agent_manager.cpp +++ b/agent_runtime_framework/frameworks/js/napi/agent_manager/src/js_agent_manager.cpp @@ -535,7 +535,7 @@ void DoConnectAgentExtensionAbility(napi_env env, GetAgentManagerErrorMsg(innerErrCode, AgentManagerErrorOperation::CONNECT_AGENT_EXTENSION)); // Sync connect failure: reject primary + staged low-code tasks (pendingLowCodeReuseTasks_) + remove // the connection, else Mechanism-A tasks hang (host never connects -> drain never runs). - connection->RejectConnectAndCleanup(env, error, /*hasPrimaryTask=*/true); + connection->RejectConnectAndCleanup(env, error); } } @@ -1129,28 +1129,38 @@ napi_value JsAgentManager::OnDisconnectAgentExtensionAbility(napi_env env, size_ } connection->SetDisconnecting(true); - connection->SetDisconnectAsyncTask(disconnectTaskShared); + if (!connection->SetDisconnectAsyncTask(disconnectTaskShared)) { + // A disconnect is already pending; resolve as duplicate to avoid orphaning it. + TAG_LOGI(AAFwkTag::SER_ROUTER, "Disconnect already pending, resolve as duplicate"); + disconnectTaskShared->ResolveWithNoError(env, CreateJsUndefined(env)); + return result; + } auto innerErrCode = std::make_shared(ERR_OK); auto execute = std::make_unique( [connection, innerErrCode]() { TAG_LOGD(AAFwkTag::SER_ROUTER, "Execute disconnect, connectionId: %{public}s", std::to_string(connection->GetConnectionId()).c_str()); *innerErrCode = AgentConnectionManager::GetInstance().DisconnectAgentExtensionAbility(connection); - if (*innerErrCode != ERR_OK) { - connection->SetDisconnecting(false); - connection->SetDisconnectAsyncTask(nullptr); - } + // Keep IsDisconnecting true until the JS-thread complete settles (prevents re-entrant overwrite); + // do not null disconnectAsyncTask_ here — it is settled via TakeDisconnectAsyncTask(). }); auto complete = std::make_unique( - [innerErrCode, disconnectTaskShared, connection](napi_env env, NapiAsyncTask &task, int32_t status) { + [innerErrCode, connection](napi_env env, NapiAsyncTask &task, int32_t status) { if (*innerErrCode == ERR_OK) { return; } TAG_LOGE(AAFwkTag::SER_ROUTER, "Disconnect failed: %{public}d", *innerErrCode); - disconnectTaskShared->Reject(env, - CreateJsError(env, static_cast(GetJsErrorCodeByNativeError(*innerErrCode)), - GetAgentManagerErrorMsg(*innerErrCode, AgentManagerErrorOperation::DISCONNECT_AGENT_EXTENSION))); + // Take ownership: if already settled by HandleOnAbilityDisconnectDone, Take returns nullptr + // and we skip — no double-settle. + if (auto pending = connection->TakeDisconnectAsyncTask(); pending != nullptr) { + pending->Reject(env, + CreateJsError(env, static_cast(GetJsErrorCodeByNativeError(*innerErrCode)), + GetAgentManagerErrorMsg( + *innerErrCode, AgentManagerErrorOperation::DISCONNECT_AGENT_EXTENSION))); + } + // Reset on the JS thread after settling to keep IsDisconnecting true during the settle window. + connection->SetDisconnecting(false); DrainReconnectPendingTasksToExistingConnection(env, connection); });