Change callback notifiers to run on thread due to avoid deadlock

In library, get*Log API call in callback function can make
deadlocks into callback notifier in server.
This can easily occur In situation that a bunch of logs are occuring.

Assume that client is in callback and just called get*Log API and
server just started to handle next audit logs at that moment.

Client will be waiting until server take care of an API request.
And then, server will be waiting after processing audit logs until
client can receive its message.
Server and client get waiting for each other.

Signed-off-by: Sungbae Yoo <sungbae.yoo@samsung.com>
Change-Id: I9b6c38db5648adc26310ab6086fd0354417ef3f8

diff --git a/common/audit/audit-logger.h b/common/audit/audit-logger.h
index 0ca4a0983b08342a811cbff735dbafe451979732..b295971428aa3771da7de9f04c0e3ec101fcd808 100644 (file)
--- a/common/audit/audit-logger.h
+++ b/common/audit/audit-logger.h
@@ -41,7 +41,7 @@ public:
                logs.clear();
        }
 
-       void setCallback(std::function<void(T&)> cb)
+       void setCallback(std::function<void(const size_t)> cb)
        {
                callback = cb;
        }
@@ -58,7 +58,7 @@ public:
                if (builder.isCompleted()) {
                        logs.push_back(builder.pop());
                        if (callback)
-                               callback(logs.back());
+                               callback(logs.size() - 1);
                        flag = true;
                }
                return flag;
@@ -67,7 +67,7 @@ public:
 private:
        std::vector<T> logs;
        AuditLogBuilder<T> builder;
-       std::function<void(T&)> callback;
+       std::function<void(const size_t)> callback;
 };
 
 #endif //!__AUDIT_TRAIL_AUDIT_LOGGER_H__

diff --git a/common/audit/audit-message-parser.cpp b/common/audit/audit-message-parser.cpp
index e750815a4b0f2badb27bdb0bfad7388585f682e3..1dc136d3d7a4663b103df50a849364ccc6509933 100644 (file)
--- a/common/audit/audit-message-parser.cpp
+++ b/common/audit/audit-message-parser.cpp
@@ -50,28 +50,35 @@ AuditMessageParser::~AuditMessageParser()
 
 void AuditMessageParser::parse()
 {
-       std::lock_guard<std::mutex> lock(nlLock);
        while (1) {
+               int type = 0;
+               std::string log;
+
                try {
+                       std::lock_guard<std::mutex> lock(nlLock);
+
                        auto msg = nl.recv(MSG_DONTWAIT);
 
-                       int type = msg.first;
-                       std::string log(msg.second.begin(), msg.second.end());
+                       type = msg.first;
+                       log = std::string(msg.second.begin(), msg.second.end());
+               } catch (runtime::Exception &e) {
+                       break;
+               }
 
-                       if (systemLogs.addMessage(type, log)) {
-                               auto &parsedSystemLogs = systemLogs.get();
+               if (systemLogs.addMessage(type, log)) {
+                       auto &parsedSystemLogs = systemLogs.get();
 
-                               if ((parsedSystemLogs[parsedSystemLogs.size() - 1].tag.size() != 0) &&
-                                       (parsedSystemLogs[parsedSystemLogs.size() - 1].tag.compare("smack") != 0)) {
-                                       tagStatistics.addCount(parsedSystemLogs[parsedSystemLogs.size() - 1].tag);
+                       if ((parsedSystemLogs[parsedSystemLogs.size() - 1].tag.size() != 0) &&
+                               (parsedSystemLogs[parsedSystemLogs.size() - 1].tag.compare("smack") != 0)) {
+                               tagStatistics.addCount(parsedSystemLogs[parsedSystemLogs.size() - 1].tag);
 
-                                       if (parsedSystemLogs[parsedSystemLogs.size() - 1].action.systemCall != 0)
-                                               syscallStatistics.addCount(parsedSystemLogs[parsedSystemLogs.size() - 1].action.systemCall);
-                               }
+                               if (parsedSystemLogs[parsedSystemLogs.size() - 1].action.systemCall != 0)
+                                       syscallStatistics.addCount(parsedSystemLogs[parsedSystemLogs.size() - 1].action.systemCall);
                        }
+                       break;
+               }
 
-                       userLogs.addMessage(type, log);
-               } catch (runtime::Exception &e) {
+               if (userLogs.addMessage(type, log)) {
                        break;
                }
        }

diff --git a/lib/audit-trail/system-log.cpp b/lib/audit-trail/system-log.cpp
index af213c3f325d37ef63fb45a03d8610ef38cfcc06..a5fdf8315697a9177033b208e314d2b48738a8c8 100644 (file)
--- a/lib/audit-trail/system-log.cpp
+++ b/lib/audit-trail/system-log.cpp
@@ -320,7 +320,7 @@ int audit_trail_add_system_log_cb(audit_trail_h handle,
                                [callback, user_data, &client] (std::string name, int position)
                                {
                                        auto manager = client.createInterface<LogManagement>();
-                                       auto log(manager.getSystemLog(position - 1));
+                                       auto log(manager.getSystemLog(position));
                                        callback(&log, user_data);
                                });
 

diff --git a/lib/audit-trail/user-log.cpp b/lib/audit-trail/user-log.cpp
index 8a29a0b4b41035538728c996e979ecc0051473ee..b6c812dd1ec37dd235e156d5c73ce3c096b1f4c7 100644 (file)
--- a/lib/audit-trail/user-log.cpp
+++ b/lib/audit-trail/user-log.cpp
@@ -120,7 +120,7 @@ int audit_trail_add_user_log_cb(audit_trail_h handle,
                                [callback, user_data, &client] (std::string name, int position)
                                {
                                        auto manager = client.createInterface<LogManagement>();
-                                       auto log(manager.getUserLog(position - 1));
+                                       auto log(manager.getUserLog(position));
                                        callback(&log, user_data);
                                });
 

diff --git a/server/log-management.cpp b/server/log-management.cpp
index 64c388765ac072fa9cc0f89f92d3d6a6d2ad94e3..517c5138a01c86a0fde431a1c8881da260d9e04a 100644 (file)
--- a/server/log-management.cpp
+++ b/server/log-management.cpp
@@ -15,10 +15,11 @@
  */
 #include <unistd.h>
 
+#include <klay/thread-pool.h>
+
 #include "rmi/log-management.h"
 
 #define PRIVILEGE_PLATFORM "http://tizen.org/privilege/internal/default/platform"
-
 namespace AuditTrail {
 
 LogManagement::LogManagement(AuditTrailControlContext &ctx) :
@@ -35,13 +36,13 @@ LogManagement::LogManagement(AuditTrailControlContext &ctx) :
        context.createNotification("SystemLog");
 
        auto &systemLogs = context.getAuditParser().systemLogs;
-       systemLogs.setCallback([&ctx, &systemLogs] (AuditSystemLog &log) {
-               ctx.notify("SystemLog", systemLogs.size());
+       systemLogs.setCallback([&ctx] (const size_t index) {
+               ctx.notify("SystemLog", index);
        });
 
        auto &userLogs = context.getAuditParser().userLogs;
-       userLogs.setCallback([&ctx, &userLogs] (AuditUserLog &log) {
-               ctx.notify("UserLog", userLogs.size());
+       userLogs.setCallback([&ctx] (const size_t index) {
+               ctx.notify("UserLog", index);
        });
 }
 

diff --git a/server/server.cpp b/server/server.cpp
index cd89d007a28eb7eb81c50ca2fa89c02c045254b4..603703882d75ed42c1f47006854641e53c6d5ce4 100644 (file)
--- a/server/server.cpp
+++ b/server/server.cpp
@@ -25,7 +25,8 @@
 #include "server.h"
 #include "loader.h"
 
-#define VCONFKEY_AUDIT_ENABLE "db/audit/enable"
+#define VCONFKEY_AUDIT_ENABLE  "db/audit/enable"
+#define NUM_OF_NOTIFIER_THREAD 3
 
 using namespace std::placeholders;
 
@@ -37,7 +38,7 @@ std::unique_ptr<AuditTrail::RuleManagement> rule;
 } // namespace
 
 Server::Server() :
-       referenceCount(0)
+       referenceCount(0), notifier(NUM_OF_NOTIFIER_THREAD)
 {
        service.reset(new rmi::Service(SERVICE_SOCKET_PATH));
 

diff --git a/server/server.h b/server/server.h
index 148b1ea26575864e7f02005d46062579ebaa6511..b40f4ccaf82f4405a284973b93aba31384336045 100644 (file)
--- a/server/server.h
+++ b/server/server.h
@@ -50,7 +50,9 @@ public:
        template <typename... Args>
        void notify(const std::string& name, Args&&... args)
        {
-               service->notify<Args...>(name, std::forward<Args>(args)...);
+               notifier.submit([this, name, &args...]() {
+                       service->notify<Args...>(name, std::forward<Args>(args)...);
+               });
        }
 
        uid_t getPeerUid() const
@@ -104,6 +106,7 @@ private:
 
        int referenceCount;
        RuleApplyEngine ruleApplyEngine;
+       runtime::ThreadPool notifier;
 };
 
 #endif //__AUDIT_TRAIL_SERVER_H__