file sinks refactoring

include/c11log/logger.h

@@ -76,7 +76,7 @@ inline c11log::details::line_logger c11log::logger::log(c11log::level::level_enu
     {
         std::lock_guard<std::mutex> lock(mutex_);
         return details::line_logger(this, msg_level);
-    } 
+    }
     else
     {
         return details::line_logger(nullptr);

include/c11log/sinks/async_sink.h

@@ -32,7 +32,6 @@ protected:
 private:
     c11log::logger::sinks_vector_t sinks_;
     std::atomic<bool> active_ { true };
-    const std::chrono::seconds push_pop_timeout_;
     c11log::details::blocking_queue<std::string> q_;
     std::thread back_thread_;
     //Clear all remaining messages(if any), stop the back_thread_ and join it
@@ -47,7 +46,6 @@ private:
 
 inline c11log::sinks::async_sink::async_sink(const std::size_t max_queue_size)
     :q_(max_queue_size),
-     push_pop_timeout_(std::chrono::seconds(2)),
      back_thread_(&async_sink::thread_loop_, this)
 {}
 
@@ -57,19 +55,20 @@ inline c11log::sinks::async_sink::~async_sink()
 }
 inline void c11log::sinks::async_sink::sink_it_(const std::string& msg)
 {
-    q_.push(msg, push_pop_timeout_);
+    q_.push(msg);
 }
 
 inline void c11log::sinks::async_sink::thread_loop_()
 {
     std::string msg;
+    auto pop_timeout = std::chrono::seconds(1);
     while (active_)
     {
-        if (q_.pop(msg, push_pop_timeout_))
+        if (q_.pop(msg, pop_timeout))
         {
             for (auto &sink : sinks_)
             {
-                sink->log(msg, _level);
+                sink->log(msg, static_cast<level::level_enum>(_level.load()));
                 if (!active_)
                     return;
             }

include/c11log/sinks/base_sink.h

@@ -1,8 +1,7 @@
 #pragma once
 
 #include<string>
-#include<memory>
+#include<atomic>
-#include<mutex>
 
 #include "../formatters/formatters.h"
 #include "../level.h"
@@ -11,33 +10,29 @@ namespace c11log {
 namespace sinks {
 class base_sink {
 public:
-    base_sink() = default;    
+    base_sink() = default;
     base_sink(level::level_enum l):_level(l)
     {};
     virtual ~base_sink() = default;
-    
+
     base_sink(const base_sink&) = delete;
     base_sink& operator=(const base_sink&) = delete;
 
     void log(const std::string &msg, level::level_enum level)
     {
         if (level >= _level) {
-            std::lock_guard<std::mutex> lock(mutex_);
+            sink_it_(msg);
-            if (level >= _level)
-                sink_it_(msg);
         }
     };
 
     void set_level(level::level_enum level)
     {
-        std::lock_guard<std::mutex> lock(mutex_);
         _level = level;
     }
 
 protected:
     virtual void sink_it_(const std::string& msg) = 0;
-    level::level_enum _level = level::INFO;
+    std::atomic<int> _level = level::INFO;
-    std::mutex mutex_;
 };
 
 class null_sink:public base_sink {

include/c11log/sinks/file_sinks.h

@@ -2,6 +2,8 @@
 
 #include <fstream>
 #include  <iomanip>
+#include <mutex>
+
 
 #include "../logger.h"
 #include "../log_exception.h"
@@ -14,7 +16,7 @@ namespace sinks {
 /*
 * Trivial file sink with single file as target
 */
-class simple_file_sink:base_sink {
+class simple_file_sink : public base_sink {
 public:
     simple_file_sink(const std::string &filename, const std::string& extension = "txt")
     {
@@ -25,38 +27,21 @@ public:
 protected:
     void sink_it_(const std::string& msg) override
     {
+        std::lock_guard<std::mutex> lock(mutex_);
         _ofstream << msg;
         _ofstream.flush();
     }
-
+private:
+    std::mutex mutex_;
     std::ofstream _ofstream;
 };
 
 
+
 /*
- * Rotating file sinks. Close and open new file at some point
+ * Thread safe, size limited file sink
- */
+*/
-namespace details {
+class rotating_file_sink : public base_sink {
-class rotating_file_sink_base:public base_sink {
-public:
-    rotating_file_sink_base()
-    {}
-    virtual ~rotating_file_sink_base()
-    {}
-protected:
-    virtual void sink_it_(const std::string& msg) override
-    {
-        if (_should_rotate())
-            _rotate();
-        _ofstream << msg;
-        _ofstream.flush();
-    }
-    virtual bool _should_rotate() const = 0;
-    virtual void _rotate() = 0;
-    std::ofstream _ofstream;
-};
-}
-class rotating_file_sink:public details::rotating_file_sink_base {
 public:
     rotating_file_sink(const std::string &base_filename, const std::string &extension, size_t max_size, size_t max_files):
         _base_filename(base_filename),
@@ -70,37 +55,20 @@ public:
     }
 
 protected:
-    virtual void sink_it_(const std::string& msg) override
+    void sink_it_(const std::string& msg) override
     {
+        std::lock_guard<std::mutex> lock(mutex_);
         _current_size += msg.length();
-        rotating_file_sink_base::sink_it_(msg);
+        if (_current_size  > _max_size)
-    }
+        {
-
+            _rotate();
-    bool _should_rotate() const override
+            _current_size = msg.length();
-    {
-        return _current_size >= _max_size;
-    }
-
-    // Rotate old files:
-    // log.n-1.txt -> log.n.txt
-    // log n-2.txt -> log.n-1.txt
-    // ...
-    // log.txt -> log.1.txt
-    void _rotate() override
-    {
-        _ofstream.close();
-        _current_size = 0;
-        //Remove oldest file
-        for (auto i = _max_files; i > 0; --i) {
-            auto src = _calc_filename(_base_filename, i - 1, _extension);
-            auto target = _calc_filename(_base_filename, i, _extension);
-            if (i == _max_files)
-                std::remove(target.c_str());
-            std::rename(src.c_str(), target.c_str());
         }
-
+        _ofstream << msg;
-        _ofstream.open(_calc_filename(_base_filename, 0, _extension));
+        _ofstream.flush();
     }
+
+
 private:
     static std::string _calc_filename(const std::string& filename, std::size_t index, const std::string& extension)
     {
@@ -111,19 +79,42 @@ private:
             oss << filename << "." << extension;
         return oss.str();
     }
+
+
+    // Rotate old files:
+    // log.n-1.txt -> log.n.txt
+    // log n-2.txt -> log.n-1.txt
+    // ...
+    // log.txt -> log.1.txt
+    void _rotate()
+    {
+        _ofstream.close();
+        //Remove oldest file
+        for (auto i = _max_files; i > 0; --i) {
+            auto src = _calc_filename(_base_filename, i - 1, _extension);
+            auto target = _calc_filename(_base_filename, i, _extension);
+            if (i == _max_files)
+                std::remove(target.c_str());
+            std::rename(src.c_str(), target.c_str());
+        }
+        _ofstream.open(_calc_filename(_base_filename, 0, _extension));
+    }
+
     std::string _base_filename;
     std::string _extension;
     std::size_t _max_size;
     std::size_t _max_files;
     std::size_t _current_size;
     std::size_t _index;
+    std::ofstream _ofstream;
+    std::mutex mutex_;
 
 };
 
 /*
- * File sink that closes the log file at midnight and opens new one
+ * Thread safe file sink that closes the log file at midnight and opens new one
  */
-class midnight_file_sink:public details::rotating_file_sink_base {
+class midnight_file_sink:public base_sink {
 public:
     midnight_file_sink(const std::string& base_filename, const std::string& extension = "txt"):
         _base_filename(base_filename),
@@ -135,16 +126,17 @@ public:
     }
 
 protected:
-    bool _should_rotate() const override
+    void sink_it_(const std::string& msg) override
     {
-        return std::chrono::system_clock::now() >= _midnight_tp;
+        std::lock_guard<std::mutex> lock(mutex_);
-    }
+        if (std::chrono::system_clock::now() >= _midnight_tp)
-    void _rotate() override
+        {
-    {
+            _ofstream.close();
-        _midnight_tp = _calc_midnight_tp();
+            _ofstream.open(_calc_filename(_base_filename, _extension));
-        _ofstream.close();
+            _midnight_tp = _calc_midnight_tp();
-        _ofstream.open(_calc_filename(_base_filename, _extension));
+        }
-
+        _ofstream << msg;
+        _ofstream.flush();
     }
 
 private:
@@ -167,9 +159,12 @@ private:
         oss << basename << std::put_time(&now_tm, ".%Y-%m-%d.") << extension;
         return oss.str();
     }
+
     std::string _base_filename;
     std::string _extension;
     std::chrono::system_clock::time_point _midnight_tp;
+    std::ofstream _ofstream;
+    std::mutex mutex_;
 };
 }
 }

test/test.cpp

@@ -8,27 +8,36 @@
 int main(int argc, char* argv[])
 {
 
+
+    auto null_sink = std::make_shared<c11log::sinks::null_sink>();
+    auto async = std::make_shared<c11log::sinks::async_sink>(100);
+    auto fsink = std::make_shared<c11log::sinks::rotating_file_sink>("newlog", "txt", 1024*1024*10 , 2);
+
+    async->add_sink(fsink);
+
     c11log::logger logger("test");
-
-    auto screen_sink = std::make_shared<c11log::sinks::stdout_sink>();
-    auto file_sink = std::make_shared<c11log::sinks::midnight_file_sink>("logtest");
-    auto async = std::make_shared<c11log::sinks::async_sink>(1000);
-    async->add_sink(file_sink);
     logger.add_sink(async);
-    //logger.add_sink(file_sink);
 
-
+    std::atomic<uint32_t> counter { 0 };
-    auto fn = [&logger]()
+    auto counter_ptr = &counter;
+    for (int i = 0; i < 4; i++)
     {
-        logger.info() << "Hello logger!";
+        new std::thread([&logger, counter_ptr]() {
-    };
+            while (true)
-    utils::bench("test log", std::chrono::seconds(3), fn);
+            {
-    logger.info() << "bye";
+                logger.info() << "Hello from thread" << std::this_thread::get_id();
-    utils::bench("shutdown", [&async]() {
+                (*counter_ptr)++;
-        async->shutdown(std::chrono::seconds(10));
+            }
-    });
-
 
+        });
+    }
+    while (true)
+    {
+        counter = 0;
+        std::this_thread::sleep_for(std::chrono::seconds(1));
+        std::cout << "Counter = " << utils::format(counter.load()) << std::endl;
+    }
+    async->shutdown(std::chrono::seconds(10));
     return 0;
 }