From f5f70ffc2a5a59c931eed5d67520b520f8095f68 Mon Sep 17 00:00:00 2001
From: Rafal Krypa <r.krypa@samsung.com>
Date: Wed, 11 Jan 2017 14:13:36 +0100
Subject: [PATCH] Fix thread synchronization in Cynara class

Pass changes to cynaraFd and fd events to be polled via atomic variables
and over atomic_thread_fence to properly propagate changes to these
values between checking threads and communication thread.

Change-Id: I9b41a0f8e40365bc30cdd47ed04be8727521476e
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
---
 src/common/cynara.cpp       | 43 ++++++++++++++++++-------------------------
 src/common/include/cynara.h |  6 +++++-
 2 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/src/common/cynara.cpp b/src/common/cynara.cpp
index add9815..fd18f21 100644
--- a/src/common/cynara.cpp
+++ b/src/common/cynara.cpp
@@ -705,24 +705,13 @@ int CynaraAdmin::GetPrivilegeManagerMaxLevel(const std::string &label, const std
     return result;
 }
 
-Cynara::Cynara()
+Cynara::Cynara() : eventFd(eventfd(0, 0)), cynaraFd(eventFd), cynaraFdEvents(0), terminate(false)
 {
-    int ret;
-
-    ret = eventfd(0, 0);
-    if (ret == -1) {
+    if (eventFd == -1) {
         LogError("Error while creating eventfd: " << GetErrnoString(errno));
         ThrowMsg(CynaraException::UnknownError, "Error while creating eventfd");
     }
 
-    // Poll the eventfd for reading
-    pollFds[0].fd = ret;
-    pollFds[0].events = POLLIN;
-
-    // Temporary, will be replaced by cynara fd when available
-    pollFds[1].fd = pollFds[0].fd;
-    pollFds[1].events = 0;
-
     cynara_async_configuration *p_conf = nullptr;
     checkCynaraError(cynara_async_configuration_create(&p_conf),
                      "Cannot create cynara async configuration");
@@ -740,7 +729,7 @@ Cynara::Cynara()
 Cynara::~Cynara()
 {
     LogDebug("Sending terminate event to Cynara thread");
-    terminate.store(true);
+    terminate = true;
     threadNotifyPut();
     thread.join();
 
@@ -751,7 +740,7 @@ Cynara::~Cynara()
 
 void Cynara::threadNotifyPut()
 {
-    int ret = eventfd_write(pollFds[0].fd, 1);
+    int ret = eventfd_write(eventFd, 1);
     if (ret == -1)
         LogError("Unexpected error while writing to eventfd: " << GetErrnoString(errno));
 }
@@ -759,7 +748,7 @@ void Cynara::threadNotifyPut()
 void Cynara::threadNotifyGet()
 {
     eventfd_t value;
-    int ret = eventfd_read(pollFds[0].fd, &value);
+    int ret = eventfd_read(eventFd, &value);
     if (ret == -1)
         LogError("Unexpected error while reading from eventfd: " << GetErrnoString(errno));
 }
@@ -770,21 +759,22 @@ void Cynara::statusCallback(int oldFd, int newFd, cynara_async_status status)
         "Status = " << status << ", oldFd = " << oldFd << ", newFd = " << newFd);
 
     if (newFd == -1) {
-        pollFds[1].events = 0;
+        cynaraFdEvents = 0;
     } else {
-        pollFds[1].fd = newFd;
 
+        cynaraFd = newFd;
         switch (status) {
         case CYNARA_STATUS_FOR_READ:
-            pollFds[1].events = POLLIN;
+            cynaraFdEvents = POLLIN;
             break;
 
         case CYNARA_STATUS_FOR_RW:
-            pollFds[1].events = POLLIN | POLLOUT;
+            cynaraFdEvents = POLLIN | POLLOUT;
             break;
         }
     }
 
+    std::atomic_thread_fence(std::memory_order_release);
     threadNotifyPut();
 }
 
@@ -834,7 +824,10 @@ void Cynara::run()
 {
     LogInfo("Cynara thread started");
     while (true) {
+        std::atomic_thread_fence(std::memory_order_acquire);
+        struct pollfd pollFds[2] = {{eventFd, POLLIN, 0}, {cynaraFd, cynaraFdEvents, 0}};
         int ret = poll(pollFds, 2, -1);
+
         if (ret == -1) {
             if (errno != EINTR)
                 LogError("Unexpected error returned by poll: " << GetErrnoString(errno));
@@ -844,23 +837,23 @@ void Cynara::run()
         // Check eventfd for termination signal
         if (pollFds[0].revents) {
             threadNotifyGet();
-            if (terminate.load()) {
+            if (terminate) {
                 LogInfo("Cynara thread terminated");
                 return;
             }
         }
 
         // Check if Cynara fd is ready for processing
-        try {
-            if (pollFds[1].revents) {
+        if (pollFds[1].revents) {
+            try {
                 // Critical section
                 std::lock_guard<std::mutex> guard(mutex);
 
                 checkCynaraError(cynara_async_process(cynara),
                     "Unexpected error returned by cynara_async_process");
+            } catch (const CynaraException::Base &e) {
+                LogError("Error while processing Cynara events: " << e.DumpToString());
             }
-        } catch (const CynaraException::Base &e) {
-            LogError("Error while processing Cynara events: " << e.DumpToString());
         }
     }
 }
diff --git a/src/common/include/cynara.h b/src/common/include/cynara.h
index 7d78b59..5be7344 100644
--- a/src/common/include/cynara.h
+++ b/src/common/include/cynara.h
@@ -365,7 +365,11 @@ private:
     struct pollfd pollFds[2];
     std::mutex mutex;
     std::thread thread;
-    std::atomic<bool> terminate{false};
+
+    const int eventFd;
+    std::atomic<int> cynaraFd;
+    std::atomic<short> cynaraFdEvents;
+    std::atomic<bool> terminate;
 };
 
 } // namespace SecurityManager
-- 
2.7.4