Использование mutex приводит к зависанию программы

Я пытаюсь изучить программирование параллелизма на C++.

Я реализовал базовый класс стека с методами push (), pop (), top () и empty ().

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

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

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

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

#include <thread>
#include <mutex>
#include <string>
#include <iostream>
#include <vector>

using std::cin;
using std::cout;
std::mutex mtx;
std::mutex a_mtx;


class MyStack
{
    std::vector<int> stk;
public:
    void push(int val) {
        stk.push_back(val);
    }

    void pop() {
        mtx.lock();
        stk.pop_back();
        mtx.unlock();
    }

    int top() const {
        mtx.lock();
        return stk[stk.size() - 1];
    }

    bool empty() const {
        mtx.lock();
        return stk.size() == 0;
    }
};

void func(MyStack& ms, const std::string s)
{
    while(!ms.empty()) {
        mtx.unlock();
        a_mtx.lock();
        cout << s << " " << ms.top() << "\n";
        a_mtx.unlock();
        mtx.unlock();
        ms.pop();
    }

    //mtx.unlock();
}

int main(int argc, char const *argv[])
{
    MyStack ms;

    ms.push(3);
    ms.push(1);
    ms.push(4);
    ms.push(7);
    ms.push(6);
    ms.push(2);
    ms.push(8);

    std::string s1("from thread 1"), s2("from thread 2");
    std::thread t1(func, std::ref(ms), "from thread 1");
    std::thread t2(func, std::ref(ms), "from thread 2");

    t1.join();
    t2.join();

    cout << "Done\n";

    return 0;
}

Я подумал, потому что, когда стек был пуст, я не разблокировал мьютекс. Поэтому, когда я раскомментирую закомментированную строку и запускаю ее, она дает тарабарщину и segfault.

Я не знаю, где я делаю ошибку. Это правильный способ написания поточно-безопасного класса стека?

вместо ручного вызова mtx.lock() оберните его в класс RAII - std::lock_guard<std::mutex> g(mtx);. Это гарантирует правильную блокировку и разблокировку, даже если что-то вызовет исключение. Также - это на 1 строчку короче :). Я думаю, что ошибка в top(), так как вы блокируете мьютекс, а никогда разблокирует его. Обертывание в std::lock_guard должно исправить это

Fureeish 13.09.2018 18:03

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

Suraj Pal 13.09.2018 18:06

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

Useless 13.09.2018 18:06

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

Useless 13.09.2018 18:07

@SurajPal, я отредактировал свой комментарий. Обновите страницу - мои два последних предложения должны пролить свет на проблему

Fureeish 13.09.2018 18:07

Непревзойденные вызовы блокировки / разблокировки, поэтому и зависает. В любом случае, сам подход ошибочен: если вы проверите, пуст ли он (защищен мьютексом), а затем предположите что-нибудь после снятия блокировки, ваш код сломан. О, кстати, еще одна вещь: если вы используете мьютекс, всегда документируйте, какие объекты этот мьютекс должен охранять. Это заставляет задуматься над тем, чтобы избежать ошибок.

Ulrich Eckhardt 13.09.2018 18:08

@UlrichEckhardt Но из-за оператора return он не дойдет до оператора разблокировки, поэтому нет смысла разблокировать его там. Я новичок во всей этой обработке параллелизма. Поскольку я учусь, я хотел сначала начать с основ. Так что мой дизайн может быть ошибочным.

Suraj Pal 13.09.2018 18:10
1
7
725
2
Перейти к ответу Данный вопрос помечен как решенный

Ответы 2

Одна ошибка в том, что MyStack::top и MyStack::empty не разблокируют мьютекс.

Используйте std::lock_guard<std::mutex> для автоматической разблокировки мьютекса и устранения риска таких случайных блокировок. Например.:

bool empty() const {
    std::lock_guard<std::mutex> lock(mtx);
    return stk.empty();
}

И, вероятно, также необходимо заблокировать мьютекс в MyStack::push.


Другая ошибка заключается в том, что блокировка на уровне метода слишком мелкозернистая, и empty(), за которым следуют top() и pop(), не является атомарным.

Возможные исправления:

class MyStack
{
    std::vector<int> stk;
public:
    void push(int val) {
        std::lock_guard<std::mutex> lock(mtx);
        stk.push_back(val);
    }

    bool try_pop(int* result) {
        bool popped;
        {
            std::lock_guard<std::mutex> lock(mtx);
            if((popped = !stk.empty())) {
                *result = stk.back();
                stk.pop_back();
            }
        }
        return popped;
    }
};

void func(MyStack& ms, const std::string& s)
{
    for(int top; ms.try_pop(&top);) {
        std::lock_guard<std::mutex> l(a_mtx);
        cout << s << " " << top << "\n";
    }
}

@MaximEgorushkin Извините, но я боюсь, что это все еще оставляет состояние гонки в тактическом состоянии: (

Geezer 13.09.2018 18:35

@SkepticalEmpiricist Исправлено.

Maxim Egorushkin 13.09.2018 18:52

@SurajPal Добавил решение для вас.

Maxim Egorushkin 13.09.2018 18:55

@MaximEgorushkin Рад видеть. Не проголосовали немедленно.

Geezer 13.09.2018 18:56

Спасибо, @MaximEgorushkin. На ошибку, которую я делал, указал скептический эмпирик, и теперь я знаю, как на самом деле написать класс стека TS.

Suraj Pal 13.09.2018 19:03
Ответ принят как подходящий

it gives gibberish output and segfault.

еще потенциально даст вам segfault при текущей схеме синхронизации, даже если вы выберете предложенный Блокировка в стиле RAII, например:

void pop() {
    std::lock_guard<std::mutex> lock{ mtx };
    stk.pop_back();
}

int top() const {
    std::lock_guard<std::mutex> lock{ mtx };
    return stk[stk.size() - 1];
}

bool empty() const {
    std::lock_guard<std::mutex> lock{ mtx };
    return stk.size() == 0;
}

как вы не заботитесь о состояние гонки, возникающий между двумя последующими вызовами этих методов разными потоками. Например, подумайте, что происходит, когда в стеке остается один элемент, и один поток спрашивает, пуст ли он, и получает false, а затем у вас есть переключатель контекста, а другой поток получает тот же false для того же вопроса. Итак, они оба гоняются за top() и pop(). В то время как первый уже выскакивает, а затем другой пытается top(), он сделает это в ситуации, когда stk.size() - 1 дает -1. Следовательно, вы получаете segfault за попытку доступа к несуществующему отрицательному индексу стека: (

I don't know where I am doing a mistake. Is this the right way of writing a thread-safe stack class?

Нет, это неправильный путь, мьютекс только гарантирует, что другие потоки, блокирующие один и тот же мьютекс, не могут в настоящее время выполнять этот же участок кода. Если они попадут в один и тот же раздел, им будет заблокирован вход в него до тех пор, пока мьютекс не будет освобожден. Но вы вообще не блокируетесь между звонками на empty() и остальные звонки. Один поток добирается до empty(), блокируется, получает значение, затем освобождает его, а затем другой поток может свободно входить и запрашивать и вполне может получить то же значение. Что мешает ему позже войти в ваш вызов top(), и что мешает первому потоку уже быть после того же самого pop() в то время?

В этих сценариях вам нужно быть осторожным, чтобы увидеть полный объем того, что требует защиты с точки зрения синхронности. То, что здесь сломано, называется атомарность, что означает свойство «невозможно разрезать посередине». Как вы можете видеть, здесь говорится, что "Атомарность часто обеспечивается взаимным исключением," - например, с использованием мьютексов, как и вы. Не хватало того, что это было зернисто слишком мелко - слишком мал был "размер атомарной" операции. Вы должны были защищать всю последовательность empty()-top()-pop() в целом, поскольку теперь мы понимаем, что не можем отделить какую-либо часть из трех. В коде это может выглядеть примерно так, как вызов этого внутри func() и печать на cout, только если он вернул true:

bool safe_pop(int& value)
{
    std::lock_guard<std::mutex> lock{ mtx };

    if (stk.size() > 0)
    {
        value = stk[stk.size() - 1];
        stk.pop_back();
        return true;
    }

    return false;
}

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

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

Suraj Pal 13.09.2018 18:33

@SurajPal Я отредактировал, чтобы попытаться ответить на этот вопрос более четко. Дайте мне знать, если потребуется дальнейшее объяснение.

Geezer 13.09.2018 18:42

Да, теперь я понимаю, где что-то пошло не так и возникло segfault. Хотя я понял это сейчас, я все еще озадачен тем фактом, что если это не способ написать потокобезопасный класс стека, тогда что? Я отмечаю это как принятый ответ. Спасибо за помощь и указание на ошибку :).

Suraj Pal 13.09.2018 18:52

@SurajPal Правильный способ - увидеть, что именно нужно защитить в целом, которые нельзя небезопасно разделить на более мелкие части. Я добавлю еще одну правку, чтобы подробнее рассказать об этом.

Geezer 13.09.2018 18:59

@SurajPal Вам нужны функции, которые можно безопасно вызывать. Ваша функция pop может быть безопасно вызвана только в том случае, если вы знаете, что стек будет непустым после получения мьютекса. Это делает эту функцию не очень полезной.

David Schwartz 13.09.2018 19:51

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

Geezer 13.09.2018 19:53

Другие вопросы по теме