Лучший способ сделать поток реализации следующего логгера безопасным? - PullRequest
2 голосов
/ 17 марта 2019

Я хотел написать базовый поточно-безопасный регистратор с cout подобным интерфейсом.Я придумал следующий дизайн класса.Это определенно не лучший дизайн, так как при неправильном использовании он может зайти в тупик, как показано в int main().

#include <iostream>
#include <sstream>  // for string streams 
#include <mutex>
#include <memory>

typedef std::ostream&(*endl)(std::ostream&);

class BasicLogger {

public:
  enum SEVERITY {
    CRITICAL,
    ERROR,
    WARNING
  };

  explicit BasicLogger(SEVERITY _s): s(_s) {
    streamMutex.lock();
    logStream.reset(new std::ostringstream);
  }

  ~BasicLogger() {
    std::cout << logStream->str();
    streamMutex.unlock();
  }

  std::ostringstream& operator<< (const endl eof) {
    (*logStream) << eof;
    return (*logStream);
  }

  template<typename T>
  std::ostringstream& operator<< (const T& obj) {
    (*logStream) << obj;
    return (*logStream);
  }

  static std::unique_ptr<std::ostringstream> logStream;

  BasicLogger(const BasicLogger&) = delete;
  BasicLogger& operator=(const BasicLogger&) = delete;

private:
  SEVERITY s;          //TODO
  static std::mutex streamMutex;

};

/*=======================================================*/
std::unique_ptr<std::ostringstream> BasicLogger::logStream;
std::mutex BasicLogger::streamMutex;
/*=======================================================*/

int main() {

  int a = 9;
  int b = 8;

  // BasicLogger l(BasicLogger::ERROR); //Deadlock situation

  BasicLogger(BasicLogger::ERROR) << "Linux" << " " << a << " " << b << std::endl;
  BasicLogger(BasicLogger::ERROR) << "MyMachine";
  BasicLogger(BasicLogger::ERROR) << std::endl;

}

Ответы [ 2 ]

2 голосов
/ 17 марта 2019

Вы блокируете свой мьютекс в конструкторе и разблокируете его в деструкторе. Следовательно, невозможно создать более одного экземпляра BasicLogger одновременно.

BasicLogger l(BasicLogger::ERROR); вызовет конструктор, тем самым получив блокировку. Деструктор не будет вызываться до тех пор, пока l не выйдет из области видимости, что означает, что мьютекс остается заблокированным до тех пор, пока l не выйдет из области видимости.

Если вы попытаетесь построить еще один BasicLocker, попытка вашего конструктора получить блокировку, которая недоступна до уничтожения l, приведет к тупику.

Когда вы создаете временные BasicLogger экземпляры, BasicLogger(BasicLogger::ERROR), конструктор вызывается, объект используется, а затем немедленно уничтожается. Следовательно, заблокированный мьютекс разблокирован.


Поскольку вы создаете независимый std::stringstream для каждого экземпляра BasicLogger, вам требуется блокировка, защищающая доступ к std::stringstream, чтобы несколько потоков могли выполнять запись в один и тот же регистратор. Следовательно, вы должны иметь мьютекс в каждом экземпляре.

Вам также необходим статический мьютекс, который защищает одновременный доступ к std::cout. Блокировка получается, когда журнал распечатывается и сразу же освобождается. Конечно, это требует, чтобы все обращения к std::cout осуществлялись через BasicLogger.

class BasicLogger {
public:
    BasicLogger() = default;
    ~BasicLogger() {
        std::lock_guard<std::mutex> lLock(localMutex); /* the order of locking is important */
        std::lock_guard<std::mutex> gLock(globalMutex);
        std::cout << stream.str();
    }

    /* TODO: satisfying the rule of 5 */

    template <class T>
    BasicLogger& operator<< (const T& item) {
        std::lock_guard<std::mutex> lLock(localMutex);
        stream << item;
        return *this;
    }

private:
    std::ostringstream stream;
    std::mutex localMutex;
    static std::mutex globalMutex;
};
1 голос
/ 17 марта 2019

Я бы рассмотрел только один operator<< и только в этой функции-члене, блокирующей мьютекс.Так что удерживайте блокировку только тогда, когда вы собираетесь писать.

И вместо статической переменной (которая в основном совпадает с глобальной переменной, поэтому вы не можете иметь несколько регистраторов) для хранения std::ostringstream, иметь переменную-член, содержащую std::ostream&.Это будет означать, что при записи нескольких вещей через несколько BasicLogger они будут казаться смешанными, но это уже было проблемой, когда несколько потоков пишут через один и тот же BasicLogger.

Чтобы устранить проблему, которая выглядит следующим образом:

BasicLogger l;

// Thread 1:
l << 1 << 2;

// Thread 2:
l << 3 << 4;

// Output is one of:
1234
1324
1342
3124
3142
3412
// Ideally it should only be
1234
3412
// (Pretend `1` is something like "x is: " and `3` is "y is: ")
// (You wouldn't want "x is: {y}" or "x is: y is: {x} {y}")

У вас может быть одна функция, которая записывает много вещей, а затем блокирует их, принимая аргументы с переменным числом аргументов.(Записано как BasicLogger::write в моем примере)

Это будет выглядеть так:

#include <iostream>
#include <utility>
#include <mutex>
#include <thread>

class BasicLogger {
public:
    enum SEVERITY {
        CRITICAL,
        ERROR,
        WARNING
    };

    // Consider logging to std::cerr by default instead
    explicit BasicLogger(SEVERITY s = BasicLogger::ERROR, std::ostream& out = std::cout)
        : severity(s), output(&out) {}

    explicit BasicLogger(std::ostream& out = std::cout)
        : severity(BasicLogger::ERROR), output(&out) {}

    BasicLogger(const BasicLogger&) = default;

    template<typename T>
    BasicLogger& operator<<(T&& obj) {
        std::lock_guard<std::mutex> lock(stream_mutex);
        (*output) << std::forward<T>(obj);
        return *this;
    }

    template<typename... T>
    void write(T&&... obj) {
        std::lock_guard<std::mutex> lock(stream_mutex);
        ((*output) << ... << std::forward<T>(obj));
    }

    std::ostream& get_output() noexcept {
        return *output;
    }
    const std::ostream& get_output() const noexcept {
        return *output;
    }

    BasicLogger& operator=(const BasicLogger&) = default;

    SEVERITY severity;
private:
    std::ostream* output;
    static std::mutex stream_mutex;
};

std::mutex BasicLogger::stream_mutex;


int main() {
    BasicLogger l(std::cerr);
    int x = 0, y = 1;

    std::thread t1([&]() {
        l.write("x is: ", x, '\n');
    });
    std::thread t2([&]() {
        l.write("y is: ", y, '\n');
    });
    t1.join();
    t2.join();
}

Или у вас может быть даже operator<<(std::tuple<T...>), а вместо l.write(...), l << std::tie(...).

Но обратите внимание на различия между этим и вашим классом.Ваш класс будет писать только один раз, используя пробел, чтобы иметь временную ostringstream, тогда как это записывает непосредственно в нужную ostream несколько раз.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...