Помогите с потоками C++

117
30 июня 2019, 03:50

Работаю с потоками первый раз. Подскажите как решить проблему. Мне нужно в первом потоке сгенерировать строку и записать ее в list, а в другом принять эту строку, отсортировать и записать в другой list. Проблема в том, что программа работает не всегда корректно, то createList сработает два раза, а потом только sortList, то еще как-то. Как можно это решить?

#include<iostream> 
#include<cstdlib> 
#include<vector> 
#include<mutex> 
#include<thread> 
#include<algorithm>
#include<string>
#include<list>
using namespace std;
const int elementsCount = 10;
void PrintList(list<string> list_)
{
    cout << "LIST" << endl;
    for (list<string>::iterator it = list_.begin(); it != list_.end(); it++)
    {
        cout << *it << ' ';
    }
    cout << endl;
}
void createList(list<string> &l, mutex& m_arr, mutex& m_out) {
    srand(time(NULL));
    int count;
    string str;
    char str_char[1];
    for (int i = 0; i < elementsCount; i++) {
        m_arr.lock();
        count = rand() % 8 + 1;
        for (size_t i = 0; i < count; i++)
        {
            str_char[0] = char(rand() % 26 + 0x61);
            str += string(str_char, sizeof(str_char));
        }
        l.push_back(str);
        str.clear();
        m_arr.unlock();
        m_out.lock();
        cout << "1 ------ Create " << l.back() << "\n";
        m_out.unlock();
    }
}
void sortList(list<string> &l, list<string> &l2, mutex& m_arr, mutex& m_out) {
    int i = 0,j=0;
    string str;
    while (i < elementsCount) {
        m_arr.lock();
        if (l.size() > j) {
            str = l.back();
            sort(str.begin(), str.end());
            l2.push_back(str);
            j++;
            m_out.lock();
            cout << " Sort " << l2.back() << "\n";
            m_out.unlock();
            i++;
        }
        m_arr.unlock();
    }
}
int main() {
    mutex m_arr, m_out;
    list<string> l1, l2;
    thread push_thread(createList, ref(l1), ref(m_arr), ref(m_out));
    thread pop_thread(sortList, ref(l1), ref(l2), ref(m_arr), ref(m_out));
    if (push_thread.joinable()) push_thread.join();
    if (pop_thread.joinable()) pop_thread.join();
    PrintList(l1);
    PrintList(l2);
    system("pause");
    return 0;
}
Answer 1

Так, ну у вас тут ситуация гонки. Вы когда проверяете thread.joinable, может сложиться так, что тред еще не закончен, следовательно, join вы не сделаете - а значит, вы просто не дождетесь того момента, когда тред отработает - и выдадите пустой список.

Решение - уберите проверку на joinable, она лишняя.

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

const int elementsCount = 10

- глобальная переменная - уже сама по себе, признак плохого дизайна и предвестник проблем в будущем, так здесь она у вас еще и не того типа - как количество элементов может быть отрицательным? Используйте size_t. Да, замечание кажется мелким, но если потом вы сотворите такое в большом, переносимом между архитектурами cpu проекте, вам будет больно.

str += string(str_char, sizeof(str_char));

- str_char можно использовать в виде просто char, не перегружая программу лишним массивом и не заставляя string каждый раз копировать из него

rand() % 26

- так ограничивать диапазон RAND неправильно, вы на выходе получите вместо почти-равномерного распределения ерунду.

list<string>::iterator

- раз уж используете c++11, пользуйтесь ranged for или auto

Вместо явных lock и unlock на мьютексе следует использовать lock_guard

Answer 2

Ваш код ужасен. Несколько моментов:

  • Вы захватывает мьютексы на слишком длинные периоды, в которых нет смысла. Мьютекс нужно захватывать ровно на то время, которое он нужен.
  • У Вас идёт перекрытие имён (i), а также сами имена ужасы.
  • Проверка на joinable() избыточна и совершенно не нужна.
  • Зачем использовать list? Его интерфейс для этой задачи ужасен.

Теперь по логике кода: у Вас один поток заполняет массив, а второй читает последний элемент массива. Разве проблема не очевидна? Что будет, если первый поток добавит 10 элементов, перед тем, как второй будет иметь шанс отработать? Правильно, второй поток обработает 10 одинаковых элементов.

Такие вещи делаются через условные переменные (condition variables), а не так как делаете Вы. Но Ваш код тоже можно сделать рабочим, для этого достаточно заменить str = l.back();, на вот такой код:

auto itemIter = l.cbegin();
advance(itemIter, j);
str = *itemIter;

Который выглядел бы вот так, если бы использовался vector:

str = l[j];

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

READ ALSO
Проверка на нажатия QPushButton
c++

Проверка на нажатия QPushButton

Есть вектор: static QVector<QString>pages={"1","2","3","4","5"}; Нужно чтобы при каждой нажатии QPushButton в textBrowser выводилось по одному элементу вектора

 101
Программа поиска и замены на Python или С++
c++

Программа поиска и замены на Python или С++

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

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

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

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

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

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

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

 152