Условные переменные - где ошибка?

129
30 июня 2019, 03:20

Опять у меня вопрос из-за чужого вопроса - на этот раз вот этого.

Я захотел попробовать свои силы в параллельном программировании, и решить задачу (один поток добавляет в лист строки, другой их оттуда берет, сортирует и записывает в свой поток) через условные переменные. Оно вроде работает, но исредка затыкается на

......
<-- String consumed
<-- Wait string produced
--> String produced
--> Wait string consumed

и стоит. Где я дурак? Если я пытаюсь добавить условие к условной переменной, получается только хуже.

Вот мой код:

#include <list>
#include <string>
#include <iostream>
#include <iomanip>
#include <thread>
#include <condition_variable>
#include <algorithm>
#include <ctime>
using namespace std;
constexpr int ELEMENTS = 10;
void printList(const list<string>& l) {
    cout <<"LIST:\n";
    for(const auto& s: l) cout << s << " ";
    cout << endl;
    }
string new_string() {
    string s;
    for(int i = rand()%8+1; i > 0; --i)
        s += rand()%26+'a';
    return s;
    }
// Сигналы о том, что строка готова и что обработана
condition_variable strReady, strHandled;
mutex mReady, mHandled;
void createList(list<string>& l) {
        {
        unique_lock lck(mHandled); // Ждем запуска второго потока
        strHandled.wait(lck);      // Без проверок, так как заведомо знаем,
        }                              // что он один
    for(int i = 0; i < ELEMENTS; ++i) {
        // Начинает создавать
        string s = new_string();
        l.push_back(s);
        cout << "--> String produced" << endl;
        strReady.notify_one();     // Уведомляем о готовности строки
        cout << "--> Wait string consumed" << endl;
        unique_lock lck(mHandled); // и ждем разрешения работать
        strHandled.wait(lck);      // Без проверок, так как заведомо знаем,
        }                              // что поток обработчика единственный
    }
void handleList(list<string>& l1, list<string>& l2) {
    strHandled.notify_one();       // Сообщаем о запуске, можно работать
    for(int i = 0; i < ELEMENTS; ++i) {
        string s;
        // Ждет сигнала
            {
            unique_lock lck(mReady);
            strReady.wait(lck);    // Без проверок, так как заведомо знаем,
            // что поток создателя единственный
            s = l1.back();
            cout << "<-- String consumed" << endl;
            }
        strHandled.notify_one();   // Строка скопирована, сообщаем, что
        // можно работать дальше
        sort(s.begin(),s.end());
        l2.push_back(s);
        cout << "<-- Wait string produced" << endl;
        }
    }
int main() {
    srand(time(0));
    list<string> l1, l2;
    thread t1(createList,ref(l1));
    thread t2(handleList,ref(l1),ref(l2));
    t1.join();
    t2.join();
    printList(l1);
    printList(l2);
    }

Еще непонятка - есть srand(time(0)), но строки всегда одни и те же.

Answer 1

У Вас несколько проблем с кодом, разной степени проблемности. Первое, какие-то бесполезные прологи:

{
    unique_lock lck(mHandled); // Ждем запуска второго потока
    strHandled.wait(lck);      // Без проверок, так как заведомо знаем,
}  

И strHandled.notify_one(); — убрать. Эти прологи не нужны. Второе:

Без проверок, так как заведомо знаем, что поток обработчика единственный

Проверки нужны, т.к. существует spurious wakeup. Т.е. поток можно проснуться не потому, что получил сигнал, а просто потому что. Так что дополнительные проверки нужны всегда.

Третье, у Вас в коде гонка, а значит UB. В первом потоке Вы делаете l.push_back(s); без защиты мьютекса, что создаёт гонку с вот этой строкой из второго потока: s = l1.back();. Закрывайте добавление строки в список мьютексом mReady и уходит.

А вообще, т.к. Вы пытаетесь полностью сериализовать 2 потока, т.е. сделать их последовательно выполнимыми, Вам достаточно одного мьюеткса — не нужно тут два, ресурс-то один.

Четвёртое, и самое главное: если strReady.notify_one(); выполняется и тут же идёт переключение на другой поток, т.е. wait не успевает отработать, тогда весь код второго потока может успеть отработать, включая strHandled.notify_one();, что приведёт к тому, что сигнал улетит во вселенную и первый поток никогда не узнает о том, что он был. Расставьте правильно мьютексы, чтобы исключить эту ситуацию, тогда зависания должны прекратиться.

Но чтобы код стал корректным, нужно применить все замечания.

Минимально изменив изначальный код, можно получить что-то такое:

condition_variable strReady, strHandled;
mutex mGuard;
void createList(list<string>& l) {
    for(int i = 0; i < ELEMENTS; ++i) {
        string s = new_string();
        unique_lock lck(mGuard);
        l.push_back(s);
        cout << "--> String produced" << endl;
        strReady.notify_one(); 
        cout << "--> Wait string consumed" << endl;
        strHandled.wait(lck);      
    } 
}
void handleList(list<string>& l1, list<string>& l2) {
    size_t processed{0};
    for(int i = 0; i < ELEMENTS; ++i) {
        string s;
        {
            unique_lock lck(mGuard);
            strReady.wait(lck, [&](){ return processed < l1.size(); });
            s = l1.back();
            ++processed;
            cout << "<-- String consumed" << endl;
        }
        strHandled.notify_one();  
        sort(s.begin(), s.end());
        l2.push_back(s);
        cout << "<-- Wait string produced" << endl;
    }
}

Из очевидных минусов: нет уведомления потока производителя, что потребитель действительно потребил. Из-за этого приходится держать мьютекс и для вывода, и для уведомления, а также нет защиты от «перепроизводства» из-за spurious wakeup, но идея должна быть ясна.

Answer 2

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

#include <list>
#include <string>
#include <iostream>
#include <iomanip>
#include <thread>
#include <condition_variable>
#include <algorithm>
#include <ctime>
using namespace std;
constexpr int ELEMENTS = 20;
void printList(const list<string>& l) {
    cout <<"LIST:\n";
    for(const auto& s: l) cout << s << " ";
    cout << endl;
}
string new_string() {
    string s;
    for(int i = rand()%8+1; i > 0; --i)
        s += rand()%26+'a';
    return s;
}
condition_variable strReady, strHandled;
mutex mReady, mHandled;
bool isReady = false;
void createList(list<string>& l) {
    srand(time(0));
    for(int i = 0; i < ELEMENTS; ++i) {
        string s = new_string();
        l.push_back(s);
        cout << "--> String produced\n";
        isReady = true;
        strReady.notify_all(); 
        cout << "--> Wait string consumed\n";
        unique_lock lck(mHandled); 
        strHandled.wait(lck,[]() {return !isReady;}); 
    }
}
void handleList(list<string>& l1, list<string>& l2) {
    for(int i = 0; i < ELEMENTS; ++i) {
        string s;
        {
            unique_lock lck(mReady);
            strReady.wait(lck,[]() {return isReady;});
            s = l1.back();
            cout << "<-- String consumed\n";
        }
        isReady = false;
        strHandled.notify_all(); 
        sort(s.begin(),s.end());
        l2.push_back(s);
        cout << "<-- Wait string produced\n";
    }
}
int main() {
    list<string> l1, l2;
    thread t1(createList,ref(l1));
    thread t2(handleList,ref(l1),ref(l2));
    t1.join();
    t2.join();
    printList(l1);
    printList(l2);
}
READ ALSO
Чтения слов из файла

Чтения слов из файла

Задание следующие: пользователь вводит адрес директории и ключевое слово, моя задача найти все текстовые файлы в этой директории, которые...

153
Цикл for проходящий по итераторам

Цикл for проходящий по итераторам

Проблема: Синтаксическая конструкция цикла for для итераторов стандартна

182
Преобразование Бокса — Мюллера

Преобразование Бокса — Мюллера

Хочу воспользоваться первым вариантом этого метода, чтобы генерировать псевдослучайные числа с нормальным распределением

118
Запрет ввода символов в Linux консоле на C++

Запрет ввода символов в Linux консоле на C++

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

121