Re: [devel] [PATCH 1/1] log: fix segmentation fault in log agent [#3137]

[Minh Hon CHAU]

Hi Vu,

Ack(review).

Thanks,
Minh
Quoting Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>:

log agent did not protect the resource `unacked_invocations_ list` from
accessing by multiple threads, so caused segmentation fault.

This patch introduces a mutex in order to synchronize the access to that
common resource.
---
 src/log/agent/lga_client.cc |  2 +-
 src/log/agent/lga_client.h  | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/log/agent/lga_client.cc b/src/log/agent/lga_client.cc
index cdc54904a..2eb37a0f7 100644
--- a/src/log/agent/lga_client.cc
+++ b/src/log/agent/lga_client.cc
@@ -86,7 +86,7 @@ LogClient::~LogClient() {
   }

   stream_list_.clear();
-  unacked_invocations_.clear();
+  CleanUnackedList();

   // Free the client handle allocated to this log client
   if (handle_ != 0) {
diff --git a/src/log/agent/lga_client.h b/src/log/agent/lga_client.h
index f5fa6faa4..e6e2c911e 100644
--- a/src/log/agent/lga_client.h
+++ b/src/log/agent/lga_client.h
@@ -174,13 +174,18 @@ class LogClient {
   // get acknowledgement from it.
   void KeepTrack(SaInvocationT inv, uint32_t ack_flags) {
     if (ack_flags != SA_LOG_RECORD_WRITE_ACK) return;
+    base::Lock scope_lock{mutex_unacked_list_};
     unacked_invocations_.push_back(inv);
   }

   // Got an acknowledgment, so remove from the track list.
-  void RemoveTrack(SaInvocationT inv) { unacked_invocations_.remove(inv); }
+  void RemoveTrack(SaInvocationT inv) {
+    base::Lock scope_lock{mutex_unacked_list_};
+    unacked_invocations_.remove(inv);
+  }

   void NotifyClientAboutLostInvocations() {
+    base::Lock scope_lock{mutex_unacked_list_};
     for (const auto& i : unacked_invocations_) {
       TRACE("The write async with this invocation %lld has been lost", i);
       // the below memory will be freed by lga_msg_destroy(cbk_msg)
@@ -232,6 +237,11 @@ class LogClient {
   // Invoke the registered callback
   void InvokeCallback(const lgsv_msg_t* msg);

+  void CleanUnackedList() {
+    base::Lock scope_lock{mutex_unacked_list_};
+    unacked_invocations_.clear();
+  }
+
   // Delete all messages from the mailbox
   static bool ClearMailBox(NCSCONTEXT, NCSCONTEXT);

@@ -290,6 +300,10 @@ class LogClient {
   // If cluster goes to headless, log agent will inform to log client with
   // SA_AIS_ERR_TRY_AGAIN code for these invocations.
   std::list<SaInvocationT> unacked_invocations_{};
+
+  // To protect the `unacked_invocations_` list.
+  base::Mutex mutex_unacked_list_{};
+
   // LOG handle (derived from hdl-mngr)
   SaLogHandleT handle_;

--
2.17.1

_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel