C++ 11 Нитевидный полиморфизм с меньшей детализацией

Question

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

class Logger 
{ 
public: 
    virtual ~Logger(); 

    LogSeverity GetSeverity() const; 
    void SetSeverity(LogSeverity s); 
protected: 
    std::mutex mutex; 
private: 
    LogSeverity severity; 
}; 

void Logger::SetSeverity(LogSeverity s) 
{ 
    std::lock_guard<std::mutex> lock(mutex); 
    severity = s; 
} 

LogSeverity Logger::GetSeverity() const 
{ 
    std::lock_guard<std::mutex> lock(mutex); 
    return severity; 
} 

void Logger::SetSeverity(LogSeverity s) const 
{ 
    std::lock_guard<std::mutex> lock(mutex); 
    severity = s; 
} 

// StreamLogger inherits from Logger 

void StreamLogger::SetStream(ostream* s) 
{ 
    std::lock_guard<std::mutex> lock(mutex); 
    stream = s; 
} 

ostream* StreamLogger::GetStream() const 
{ 
    std::lock_guard<std::mutex> lock(mutex); 
    return stream; 
} 

Однако весь доступ общественности к классу требует этого чрезвычайно избыточной блокировки.

Два варианта я вижу:

1) Caller этих публичной функции блокирует весь объект, используя семафор в классе

Logger l = new Logger(); 
std::lock_guard<std::mutex> lock(l->lock()); 
l->SetSeverity(LogDebug); 

2) замок обертка вокруг каждой переменной в классе

template typename<T> struct synchronized 
{ 
public:  
    synchronized=(const T &val); 
    // etc.. 
private: 
    std::mutex lock; 
    T v; 
}; 

class Logger 
{ 
private: 
    synchronized<LogSeverity> severity; 
}; 

Однако это решение очень ресурсоемкое, блокировка для каждого элемента.

У меня на правильном пути или есть что-то, что мне не хватает?

Answer 1

Предположим, что вам необходимо получить доступ к этим сеттерам и геттерам в разных потоках.

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

void tYourClass::thread_1() 
{ 
    .. 
    m_streamLogger.SetStream(/*new stream*/); 
} 

void tYourClass::thread_2() 
{ 
    ostream *stream = m_streamLogger.GetStream(); 
    // access the returned stream 
    // stream->whatever() 
} 

В этом случае, прямо между вы получите поток ручку и получить доступ к нему, другой поток пинки в и установите поток. Что случится? Вы получаете сообщение «dangling»: вы можете получить доступ к удаленному объекту или зарегистрировать то, что никогда не будет видно другим (в зависимости от логики внутри SetStream). Ваш замок просто не смог его защитить. Основная причина заключается в том, что вы должны блокировать операторы, которые должны выполняться как одна «атомарная» процедура, когда ни один другой поток не может забить до его завершения.

И у меня есть два предложения.

1. Не помещайте блокировки внутри сеттеров/геттеров. Это либо потому, что он не может защитить все, как указано выше, или из-за эффективности. Вы можете использовать эти методы в одном потоке. Если это так, вам не нужен замок. Как правило, сеттеры и геттеры сами не могут (и не должны) понимать, как они будут доступны. Таким образом, более разумным местом является установка блокировки в клиентском коде, где вы думаете, что многопоточность действительно задействована.
2. Не пытайтесь защитить что-либо одним замком. Предполагается, что замок должен быть как можно короче. Если вы злоупотребляете одним замком для большого количества независимых ресурсов, степень параллелизма (одно основное преимущество потока) будет скомпрометирована.

Answer 2

Прежде всего, необходимо тщательно пересмотреть возможные варианты использования:

• вы действительно должны Logger быть настолько настраивается?
• Какие свойства могут быть инициализированы во время строительства?
• Имеет смысл изменить все из них?

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

Что касается проблемы многопоточности: я не думаю, что общий регистратор - хорошая идея. Лично, я всегда предпочитаю примитивы, специфичные для нитей, в таких случаях (один журнал на поток). Зачем?

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

Если ваш компилятор поддерживает C++ 11, вышеприведенное решение в основном правильное использование thread_local, __declspec(thread) или __thread, в зависимости от того, что поддерживает ваш компилятор.

Если вы еще хотите реализовать общий регистратор, начните с обзора дизайна. Например: вы уверены, что для изменения одного свойства требуется блокировка мьютекса? Такие вещи, как severity член идеальный кандидат для std::atomic. Это может потребовать больше работы, но может быть намного быстрее.

class Logger 
{ 
    //cut 

private: 
    std::atomic<LogSeverity> severity; 
}; 

void Logger::SetSeverity(LogSeverity s) 
{ 
    severity.store(s, std::memory_order_release); 
} 

LogSeverity Logger::GetSeverity() const 
{ 
    return severity.load(std::memory_order_acquire); 
} 

std::memory_order_acquire/release это просто пример - вы можете использовать более сильное упорядочение как memory_order_seq_cst (если вам нужен общий глобальный порядок). Однако, как правило, достаточно пары acquire/release, чтобы обеспечить правильную синхронизацию между нагрузками и магазинами и небольшой бонус - они не будут создавать никаких ограждений на x86.

Если вы думаете, что можете прочитать C++ Concurrency in Action от Энтони Уильямса. Это лучший ресурс для изучения потоков, атомистики, синхронизации, порядка памяти и т. Д.

Существует также очень хорошие статьи на Bartosz Milewski's blog. Как этот: C++ atomics and memory ordering.

Если вы не знакомы с такими темами, как атомы, заборы, заказы и т. Д., Эти ресурсы очень хороши для начала.

Answer 3

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

Даже если вы сделали поток «безопасным», вы действительно хотите, чтобы один поток менял поток назначения, в то время как другой поток находится в середине протоколирования сообщений?

В данном случае это выглядит, как вы могли бы просто установить тяжесть и пару в конструкторе:

StreamLogger(LogSeverity severity, ostream& steam); 

Если число аргументов конструкторы становится неуправляемым вы можете создать строитель или фабрику или просто группу ваших аргументов в один объект:

StreamLogger(const StreamLoggerArgs& arguments); 

В качестве альтернативы вы можете выделить части регистратора, которые действительно должны быть потокобезопасными в интерфейсе. Например:

class Logger { 
    protected: 
    ~Logger(){}; 
    public: 
    virtual void log(const char* message) = 0; 
    virtual LogSeverity GetSeverity() const = 0; 
}; 

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