LINUX.ORG.RU

std::bad_function_call на указатель метода класса

 


0

1

Приветствую.

В продолжении темы Вызов по указателю метода структуры вложенной в класс

Есть обработчик очереди класса в отдельном потоке

void CQueue::Handler(void)
{
    auto Get = [this](cmd_t & cmd) {
        std::unique_lock<std::mutex> ul(mtx);
        do {
            if (!abRun) return false;
        } while (!cv.wait_for(ul, std::chrono::seconds(1), [this]{ return !pq.empty(); }));

        cmd = pq.top();
        pq.pop();

        return true;
    };

    cmd_t cmd;
    while (Get(cmd))
        if (cmd.handle)
            (this->*(cmd.handle))(cmd.id, cmd.msg);
}

В саму очередь засовываю подобным образом из другого потока поступающих команд

pQ->Put(id, std::string(msg, msgLen), &CQueue::handle_pass);

частично описание класса

class CQueue
{
private:
    struct cmd_t {
        int8_t pri;
        std::string id, msg;
        void (CQueue::*handle)(const std::string & id, std::string & msg);
    };

    std::function<bool (const cmd_t &, const cmd_t &)> cmp = [](const cmd_t & l, const cmd_t & r){ return l.pri > r.pri; };

    std::priority_queue<cmd_t, std::vector<cmd_t>, decltype(cmp)> pq;

Все работает полагаю пока команды идут настолько «медленно», что CQueue::Handler успевает встать на условную переменную, но если удается за время вызова (this->(cmd.handle))(cmd.id, cmd.msg) засунуть в очередь несколько команд, то валится ошибка std::bad_function_call при вызове (this->(cmd.handle))

Разве нельзя одновременно передавать указатель на метод класса в другой поток и обращаться к нему???

★★★

Последнее исправление: wolverin (всего исправлений: 1)

Ответ на: комментарий от alex1101

Ну вот конкретно вот это:

class CQueue
{
private:
    struct cmd_t {
        int8_t pri;
        std::string id, msg;
        void (CQueue::*handle)(const std::string & id, std::string & msg);
    };

    std::function<bool (const cmd_t &, const cmd_t &)> cmp = [](const cmd_t & l, const cmd_t & r){ return l.pri > r.pri; };

    std::priority_queue<cmd_t, std::vector<cmd_t>, decltype(cmp)> pq;

Вызывает некоторую оторопь. Поскольку, если мне не отшибает память, можно было бы сделать проще:

class CQueue
{
private:
    struct cmd_t {
        int8_t pri;
        std::string id, msg;
        void (CQueue::*handle)(const std::string & id, std::string & msg);

        // Конкретное friend для struct не нужен, но зато
        // он позволяет определить оператор сравнения в виде
        // свободной функции прямо в теле cmd_t.
        friend bool operator<(const cmd_t & l, const cmd_t & r) noexcept {
            return l.pri > r.pri;
        }
    };

    std::priority_queue<cmd_t> pq;

Но ТС своеобразен. Он уже задал кучу вопросов на LOR-е но использовать специальные теги для оформления фрагментов кода так и не научился. А за правду-матку приглашает приехать к нему в Челябинск чтобы он мог сломать вам нос.

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

eao197 ★★★★★
()

Скажите, а что мешает вам вот здесь:

void CQueue::Handler(void)
{
    auto Get = [this](cmd_t & cmd) {
        std::unique_lock<std::mutex> ul(mtx);
        do {
            if (!abRun) return false;
        } while (!cv.wait_for(ul, std::chrono::seconds(1), [this]{ return !pq.empty(); }));

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

Очевидные же вроде вещи, не?

eao197 ★★★★★
()
Ответ на: комментарий от wolverin

сразу после захвата мтх там всегда пусто, жду события так

Вы сами себе противоречите:

но если удается за время вызова (this->(cmd.handle))(cmd.id, cmd.msg) засунуть в очередь несколько команд, то валится ошибка std::bad_function_call при вызове (this->(cmd.handle))

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

eao197 ★★★★★
()
Ответ на: комментарий от eao197

когда напихивают вы конечно правы, что на cv можно было бы и не вставать, с другой стороны большую часть процессорного времени все равно в очереди пусто.

это конечно наверное мне даст минус несколько мс под нагрузкой, но пока хотелось бы понять причину ошибки std::bad_function_call

wolverin ★★★
() автор топика
Ответ на: комментарий от eao197

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

static void MQTTArrived(const char * topicName, int topicLen, char * msg, int msgLen)
{
if
...
pQ->Put(id, std::string(msg, msgLen), &CQueue::handle_xxx);
else if
...
pQ->Put(id, std::string(msg, msgLen), &CQueue::handle_yyy);

сам пут в целом тривиален

int CQueue::Put(const std::string & id, const std::string & msg, void (CQueue::*handle)(const std::string & id, std::string & msg), const int8_t pri)
{
    mtx.lock();

    pq.push({pri, id, msg, handle});
    int rt = pq.size();

    mtx.unlock();
    cv.notify_one();

    return rt;
}

wolverin ★★★
() автор топика

std::bad_function_call выбрасывается std::function::operator(), так что тут проблема точно не в самой конструкции this->*cmd.handle, надо детальнее изучать откуда выбрасывается исключение (смотреть на стектрейс).

Ещё один момент - cond_var не счиатет сколько раз на нём вызвали notify_one/all, он только будит поток(и) которые сейчас ждут, поэтому если очередь не пуста ждать на cond_var не следует - можно не дождаться.

Begemoth ★★★★★
()
Ответ на: комментарий от eao197

еще раз спасибо! помогло перетаскивание метода сравнения внутрь структуры

цикл по условной переменной переделал к такому виду, кажется ничего не забыл

void CQueue::Handler(void)
{
    auto Get = [this](cmd_t & cmd) {
        std::unique_lock<std::mutex> ul(mtx);

        LOOP:
            if (!abRun) return false;

            if (pq.empty())
            {
                cv.wait_for(ul, std::chrono::seconds(1));
                goto LOOP;
            }

        cmd = pq.top();
        pq.pop();

        return true;
    };

    cmd_t cmd;
    while (Get(cmd))
        if (cmd.handle)
            (this->*(cmd.handle))(cmd.id, cmd.msg);
}
wolverin ★★★
() автор топика
Ответ на: комментарий от x905

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

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

wolverin ★★★
() автор топика
Ответ на: комментарий от wolverin

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

какие имеющиеся очереди ты предварительно посмотрел, что решил написать свою ?

x905 ★★★★★
()

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

cobold ★★★★★
()
Последнее исправление: cobold (всего исправлений: 1)
Ответ на: комментарий от eao197

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

wolverin ★★★
() автор топика
Последнее исправление: wolverin (всего исправлений: 1)
Ответ на: комментарий от x905

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

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

wolverin ★★★
() автор топика
Ответ на: комментарий от wolverin

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

хм, но код выглядит как гавно, в сочетании со слипом

но если тебе нравится, то ждем новых нескучных тем )

x905 ★★★★★
()
Ответ на: комментарий от wolverin

но когда незная человека начинают его называть говном

Говном я вас не называл, это звиздеж.

и чем ему по жизни заниматься

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

и как он работодателя наепывает

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

но думаю вы это уже и сами поняли, иначе бы не помогали мне больше

Вы заблуждаетесь. Я пишу здесь лишь потому, что мне это ничего не стоит.

eao197 ★★★★★
()

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

Вспомни древний принцип:

Разделяй и властвуй

Должна быть отдельно очередь с прикрученными мьютексом и conditional_variable для синхронизации многопоточки, отдельно обработчики. И жизнь станет проще.

Подсказка про остановку: ее стоит реализовать как передачу пустого сообщения-терминатора в очередь вместо опроса отдельного флажка. Когда приемник получит сообщение-терминатор это и будет является сигналом, что в очередь никто больше не пишет (и ее можно удалить). Это пустое сообщение-терминатор можно смоделировать обернув свой класс сообщения в std::unique_ptr/std::optional. Вариант с опросом флажка тоже имеет право на жизнь, но но нужно рассматривать это как частный случай (не хотим обрабатывать оставшийся хвост очереди, хотим быстро завершить процесс).

dvetutnev
()
Последнее исправление: dvetutnev (всего исправлений: 1)
Ответ на: комментарий от wolverin

выставил флаг - куча очередей-потоков завершилось

Чтобы не просыпались для опроса флажка. Так получается более предсказуемое поведение. В какой-то момент эти флажки начнут размножаться и ты в них запутаешься.

dvetutnev
()
Последнее исправление: dvetutnev (всего исправлений: 2)
Ответ на: комментарий от dvetutnev

Подсказка про остановку: ее стоит реализовать как передачу пустого сообщения-терминатора в очередь вместо опроса отдельного флажка.

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

то есть, либо ее надо класть в голову очереди, специальным put, либо придавать ей наивысший приоритет и делать приоритетную очередь, либо тупо эмулировать взведением внутреннего флага завершения треда, а get из очереди будет смотреть на флаг и возвращать виртуальную мессагу - конец работы.

alysnix ★★★
()