Мой подход к потокобезопасному классу журнала ужасен?

Question

Я смотрел вокруг на различные подходы к проблеме потокового ведения журнала, но я не видел ничего подобного, поэтому я не знаю, ужасно ли это, что я не заметил, потому что я новичок в C++, потоках и iostreams. Кажется, он работает в основных тестах, через которые я прошел.

По сути, у меня есть класс Log (креатив, я знаю...), для которого настроен оператор<<, настроенный для стандартных манипуляторов, так что я могу весело передать все, что захочу.

Тем не менее, я знаю, что-то вроде:

std::cout << "Threads" << " will" << " mess" << " with" << "this." << std::endl;

будет потенциально чередоваться, когда несколько потоков записывают в cout (или везде, где указывается выходной поток журнала). Итак, я создал некоторые манипуляторы, специфичные для класса Log, которые позволяют мне делать это:

Log::log << lock << "Write" << " what" << " I" << " want" << std::endl << unlock;

Я просто хочу знать, является ли это по своей сути ужасной идеей, имея в виду, что я готов согласиться с тем, что пользователи класса Log должны быть дисциплинированы с помощью 'lock' и 'unlock'. Я подумал о том, чтобы сделать автоматическую разблокировку 'std::endl', но похоже, что это вызовет больше головной боли... Я думаю, что в любом случае должно появиться недисциплинированное использование, но если кто-нибудь сможет увидеть способ сделать такое использование вызывающим ошибки времени, это было бы хорошо.

Буду также признателен за любые предложения относительно того, как сделать мой код чище.

Вот урезанная версия класса для демонстрационных целей; у всего этого есть еще несколько конструкторов, принимающих такие вещи, как имена файлов, поэтому они не имеют отношения к вопросу.

#include <iostream>
#include <thread>
#include <fstream>

class Log{
public:
  //Constructors
  Log(std::ostream & os);
  // Destructor
  ~Log();
  // Input Functions
  Log & operator<<(const std::string & msg);
  Log & operator<<(const int & msg);
  Log & operator<<(std::ostream & (*man)(std::ostream &)); // Handles manipulators like endl.
  Log & operator<<(std::ios_base & (*man)(std::ios_base &)); // Handles manipulators like hex.
  Log & operator<<(Log & (*man)(Log &)); // Handles custom Log manipulators like lock and unlock.
  friend Log & lock(Log & log); // Locks the Log for threadsafe output.
  friend Log & unlock(Log & log); // Unlocks the Log once threadsafe output is complete.
private:
  std::fstream logFile;
  std::ostream & logStream;
  std::mutex guard;
};

// Log class manipulators.
Log & lock(Log & log); // Locks the Log for threadsafe output.
Log & unlock(Log & log); // Unlocks the Log once threadsafe output is complete.

void threadUnsafeTask(int * input, Log * log);
void threadSafeTask(int * input, Log * log);

int main(){
  int one(1), two(2);
  Log log(std::cout);
  std::thread first(threadUnsafeTask, &one, &log);
  std::thread second(threadUnsafeTask, &two, &log);
  first.join();
  second.join();
  std::thread third(threadSafeTask, &one, &log);
  std::thread fourth(threadSafeTask, &two, &log);
  third.join();
  fourth.join();
  return 0;
}

void threadUnsafeTask(int * input, Log * log){
  *log << "Executing" << " thread '" << *input << "', " << "expecting " << "interruptions " << "frequently." << std::endl;
}

void threadSafeTask(int * input, Log * log){
  *log << lock << "Executing" << " thread '" << *input << "', " << "not expecting " << "interruptions." << std::endl << unlock;
}

// Constructors (Most left out as irrelevant)
Log::Log(std::ostream & os): logFile(), logStream(logFile), guard(){
  logStream.rdbuf(os.rdbuf());
}

// Destructor
Log::~Log(){
  logFile.close();
}

// Output Operators
Log & Log::operator<<(const std::string & msg){
  logStream << msg;
  return *this;
}

Log & Log::operator<<(const int & msg){
  logStream << msg;
  return *this;
}

Log & Log::operator<<(std::ostream & (*man)(std::ostream &)){
  logStream << man;
  return *this;
}

Log & Log::operator<<(std::ios_base & (*man)(std::ios_base &)){
  logStream << man;
  return *this;
}

Log & Log::operator<<(Log & (*man)(Log &)){
  man(*this);
  return *this;
}

// Manipulator functions.
Log & lock(Log & log){
  log.guard.lock();
  return log;
}

Log & unlock(Log & log){
  log.guard.unlock();
  return log;
}

Он работает для меня на Ubuntu 12.04 g++, скомпилирован с:

g++ LogThreadTest.cpp -o log -std=c++0x -lpthread

Отсюда беззастенчиво вычеркнуты кусочки, имеющие отношение к изготовлению нестандартных манипуляторов, но не вините их за мою некомпетентную коппасту.

Answer 1

Это плохая идея. Вообразите это:

void foo()
{
    throw std::exception();
}

log << lock << "Write" << foo() << " I" << " want" << std::endl << unlock;
                          ^
                          exception!

Это оставляет ваш Log заперта. Что плохо, так как другие потоки могут ожидать блокировки. Это также происходит каждый раз, когда вы просто забыли сделать unlock, Вы должны использовать RAII здесь:

// just providing a scope
{
    std::lock_guard<Log> lock(log);
    log << "Write" << foo() << " I" << " want" << std::endl;
}

Вам нужно настроить ваш lock а также unlock методы иметь подписи void lock() а также void unlock() и сделать их членами-функциями класса Log,

---

С другой стороны, это довольно громоздко. Обратите внимание, что в C++11, используя std::cout потокобезопасен. Так что вы можете легко сделать

std::stringstream stream;
stream << "Write" << foo() << " I" << " want" << std::endl;
std::cout << stream.str();

который полностью свободен от дополнительных замков.

Answer 2

Вам не нужно явно передавать манипулятор блокировки, вы можете использовать часовой (с семантикой RAII, как говорит Ханс Пассант)

class Log{
public:
  Log(std::ostream & os);
  ~Log();

  class Sentry {
      Log &log_;
  public:
      Sentry(Log &l) log_(l) { log_.lock(); }
      ~Sentry() { log_.unlock(); }

      // Input Functions just forward to log_.logStream
      Sentry& operator<<(const std::string & msg);
      Sentry& operator<<(const int & msg);
      Sentry& operator<<(std::ostream & (*man)(std::ostream &)); // Handles manipulators like endl.
      Sentry& operator<<(std::ios_base & (*man)(std::ios_base &)); // Handles manipulators like hex.
    };

    template <typename T>
    Sentry operator<<(T t) { return Sentry(*this) << t; }
    void lock();
    void unlock();

private:
  std::fstream logFile;
  std::ostream & logStream;
  std::mutex guard;
};

Теперь пишу

Log::log << "Write" << " what" << " I" << " want" << foo() << std::endl;

будут:

1. создать временный объект Sentry
   • который блокирует объект Log
2. ... впереди каждого operator<< вызов родительского экземпляра журнала...
3. а затем выходит из области видимости в конце выражения (или если foo броски)
   • который разблокирует объект Log

---

Хотя это безопасно, это также вызывает много разногласий (мьютекс блокируется дольше, чем обычно, при форматировании сообщения). Подход с меньшим количеством конфликтов будет заключаться в том, чтобы выполнить форматирование в локальное хранилище (локальное для потока или локальное для области) без какой-либо блокировки, а затем удерживать блокировку достаточно долго, чтобы переместить ее в общую очередь журналирования.

Answer 3

Это не очень хорошая идея, потому что кто-то смертельно забудет unlock в какой-то момент все потоки зависают при следующем журнале. Существует также проблема того, что произойдет, если одно из выражений, которые вы регистрируете, выдает. (Этого не должно быть, так как вы не хотите иметь фактическое поведение в операторе log, а вещи, которые не имеют никакого поведения, не должны выбрасываться. Но вы никогда не знаете.)

Обычное решение для регистрации - использовать специальный временный объект, который захватывает блокировку в своем конструкторе и освобождает его в деструкторе (а также выполняет сброс и гарантирует, что есть завершающий '\n'). Это может быть сделано очень элегантно в C++11, используя семантику перемещения (потому что вы обычно хотите создать экземпляр временного объекта в функции, но временный объект, чей деструктор должен действовать, находится вне функции); в C++03 вам нужно разрешить копирование и убедиться, что только окончательная копия снимает блокировку.

Грубо говоря, твой Log класс будет выглядеть примерно так:

struct LogData
{
    std::unique_lock<std::mutex> myLock
    std::ostream myStream;

    LogData( std::unique_lock<std::mutex>&& lock,
             std::streambuf* logStream )
        :  myLock( std::move( lock ) )
        ,  myStream( logStream )
    {
    }

    ~LogData()
    {
        myStream.flush();
    }
};

class Log
{
    LogData* myDest;
public:
    Log( LogData* dest )
        : myDest( dest )
    {
    }
    Log( Log&& other )
        : myDest( other.myDest )
    {
        other.myDest = nullptr;
    }
    ~Log()
    {
        if ( myDest ) {
            delete myDest;
        }
    }
    Log& operator=( Log const& other ) = delete;

    template <typename T>
    Log& operator<<( T const& obj )
    {
        if ( myDest != nullptr ) {
            myDest->myStream << obj;
        }
    }
};

(Если ваш компилятор не имеет семантики перемещения, вам придется каким-то образом ее подделать. Если худший из них окажется худшим, вы можете просто сделать единственный указатель члена Log изменяемым и поместить тот же код в конструктор копирования с традиционным подпись. некрасиво, но как обходной путь...)

В этом решении у вас будет функция log, который возвращает экземпляр этого класса, либо с действительным LogData(распределяется динамически) или нулевой указатель, в зависимости от того, активно ли ведение журнала. (Можно избежать динамического выделения, используя статический экземпляр LogData у которого есть функции, чтобы начать запись в журнал и завершить ее, но это немного сложнее.)