From a87e8aeea252e5aec9e1dea0daf99562eb86b092 Mon Sep 17 00:00:00 2001
From: zhangxiaoyu <zhangxiaoyu58@huawei.com>
Date: Thu, 2 Mar 2023 20:05:49 +0800
Subject: [PATCH 31/53] fix CRI SetupPod and TearDownPod deadlock

Signed-off-by: zhangxiaoyu <zhangxiaoyu58@huawei.com>
---
 src/daemon/entry/cri/cni_network_plugin.cc    | 14 +++++++----
 .../cri/cri_pod_sandbox_manager_service.cc    |  2 ++
 src/daemon/entry/cri/network_plugin.cc        | 25 +++++++++++++------
 3 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/daemon/entry/cri/cni_network_plugin.cc b/src/daemon/entry/cri/cni_network_plugin.cc
index 1bce13f6..0fe095b6 100644
--- a/src/daemon/entry/cri/cni_network_plugin.cc
+++ b/src/daemon/entry/cri/cni_network_plugin.cc
@@ -120,8 +120,10 @@ auto CniNetworkPlugin::Name() const -> const std::string &
 
 void CniNetworkPlugin::CheckInitialized(Errors &err)
 {
-    RLockNetworkMap(err);
-    if (err.NotEmpty()) {
+    Errors tmpErr;
+    RLockNetworkMap(tmpErr);
+    if (tmpErr.NotEmpty()) {
+        err.AppendError(tmpErr.GetCMessage());
         return;
     }
 
@@ -129,9 +131,10 @@ void CniNetworkPlugin::CheckInitialized(Errors &err)
         err.SetError("cni config uninitialized");
     }
 
-    UnlockNetworkMap(err);
-    if (err.NotEmpty()) {
-        WARN("Unable to update cni config: %s", err.GetCMessage());
+    UnlockNetworkMap(tmpErr);
+    if (tmpErr.NotEmpty()) {
+        WARN("Unable to update cni config: %s", tmpErr.GetCMessage());
+        err.AppendError(tmpErr.GetCMessage());
     }
 }
 
@@ -623,6 +626,7 @@ void CniNetworkPlugin::SetUpPod(const std::string &ns, const std::string &name,
         return;
     }
 
+    err.Clear();
     RLockNetworkMap(err);
     if (err.NotEmpty()) {
         ERROR("%s", err.GetCMessage());
diff --git a/src/daemon/entry/cri/cri_pod_sandbox_manager_service.cc b/src/daemon/entry/cri/cri_pod_sandbox_manager_service.cc
index 3d183861..ee523b9c 100644
--- a/src/daemon/entry/cri/cri_pod_sandbox_manager_service.cc
+++ b/src/daemon/entry/cri/cri_pod_sandbox_manager_service.cc
@@ -705,6 +705,7 @@ auto PodSandboxManagerService::GetRealSandboxIDToStop(const std::string &podSand
         if (status->linux().namespaces().has_options()) {
             hostNetwork = (status->linux().namespaces().options().network() == runtime::v1alpha2::NamespaceMode::NODE);
         }
+        // if metadata is invalid, don't return -1 and continue stopping pod
         if (status->has_metadata()) {
             name = status->metadata().name();
             ns = status->metadata().namespace_();
@@ -831,6 +832,7 @@ auto PodSandboxManagerService::ClearCniNetwork(const std::string &realSandboxID,
         }
 
         stdAnnos.insert(std::pair<std::string, std::string>(CRIHelpers::Constants::POD_SANDBOX_KEY, netnsPath));
+        pluginErr.Clear();
         m_pluginManager->TearDownPod(ns, name, Network::DEFAULT_NETWORK_INTERFACE_NAME, realSandboxID, stdAnnos,
                                      pluginErr);
         if (pluginErr.NotEmpty()) {
diff --git a/src/daemon/entry/cri/network_plugin.cc b/src/daemon/entry/cri/network_plugin.cc
index 4a119d6b..e55db4ea 100644
--- a/src/daemon/entry/cri/network_plugin.cc
+++ b/src/daemon/entry/cri/network_plugin.cc
@@ -425,20 +425,26 @@ void PluginManager::SetUpPod(const std::string &ns, const std::string &name, con
         return;
     }
 
+    Errors tmpErr;
     std::string fullName = name + "_" + ns;
-    Lock(fullName, error);
-    if (error.NotEmpty()) {
+    Lock(fullName, tmpErr);
+    if (tmpErr.NotEmpty()) {
+        error.AppendError(tmpErr.GetCMessage());
         return;
     }
     INFO("Calling network plugin %s to set up pod %s", m_plugin->Name().c_str(), fullName.c_str());
 
-    Errors tmpErr;
     m_plugin->SetUpPod(ns, name, interfaceName, podSandboxID, annotations, options, network_settings_json, tmpErr);
     if (tmpErr.NotEmpty()) {
         error.Errorf("NetworkPlugin %s failed to set up pod %s network: %s", m_plugin->Name().c_str(), fullName.c_str(),
                      tmpErr.GetCMessage());
     }
-    Unlock(fullName, error);
+
+    tmpErr.Clear();
+    Unlock(fullName, tmpErr);
+    if (tmpErr.NotEmpty()) {
+        error.AppendError(tmpErr.GetCMessage());
+    }
 }
 
 void PluginManager::TearDownPod(const std::string &ns, const std::string &name, const std::string &interfaceName,
@@ -447,8 +453,9 @@ void PluginManager::TearDownPod(const std::string &ns, const std::string &name,
 {
     Errors tmpErr;
     std::string fullName = name + "_" + ns;
-    Lock(fullName, error);
-    if (error.NotEmpty()) {
+    Lock(fullName, tmpErr);
+    if (tmpErr.NotEmpty()) {
+        error.AppendError(tmpErr.GetCMessage());
         return;
     }
     if (m_plugin == nullptr) {
@@ -462,7 +469,11 @@ void PluginManager::TearDownPod(const std::string &ns, const std::string &name,
                      fullName.c_str(), tmpErr.GetCMessage());
     }
 unlock:
-    Unlock(fullName, error);
+    tmpErr.Clear();
+    Unlock(fullName, tmpErr);
+    if (tmpErr.NotEmpty()) {
+        error.AppendError(tmpErr.GetCMessage());
+    }
 }
 
 void NoopNetworkPlugin::Init(const std::string &hairpinMode, const std::string &nonMasqueradeCIDR, int mtu,
-- 
2.25.1