Многие из нас частенько читают статьи о багах и лучших практиках программирования, чтобы избежать возможных ошибок. Порой, ты просто знаешь, как не нужно делать, но на практике с реальной проблемой не встречаешься. Для меня такой была тема nullptr != NULL. Изначально NULL я использовала в системном программировании на Си. При переходе на C++, макрос NULL встречала только в WinAPI, в коде для null-указателей всегда использовала nullptr (все уже выучили, что nullptr != NULL). С реальными последствиями, где NULL используется вместо nullptr, мне не доводилось встречаться. Так было до замены boost::function на std::function в одном компоненте.

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

Error(s):
terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::bad_function_call> >'
  what():  call to empty boost::function

В течение трех дней подобная ситуация появилась ещё в нескольких сервисах. Значит, проблема не просто в отсутствии отписки. Нужно понять причину массовой проблемы и оценить масштаб. На днях должны сбилдить релиз‑кандидат, массовые крешдампы не допустимы. Уже несколько недель идёт обычный багфикс, никаких глобальных изменений не должно быть. Проверяю почту рассылок: на днях обновили версию одного компонента, от которого есть зависимость у всех упавших сервисов. Сроки релиза продукта и компонента разные, поэтому интеграция произошла довольно поздно. В компоненте много изменений, но все они относятся к замене boost::function на std::function.

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

connection.SetCallback(boost::function<void(bool)>());

 что эквивалентно:

connection.SetCallback(NULL);

 В реализации connection при возникновении события, вызывается колбэк-функция.

Использование boost::function в компоненте:

void SetCallback(boost::function<void(bool)> callback)
{
   ...
}

void ConnectionChanged(bool state)
{
    if (!_callback.empty())
	{
        _сallback(state);
	    // do something
	}
}

После перевода компонента на std:

void SetCallback(std::function<void(bool)> callback)
{
   ...
}

void ConnectionChanged(bool state)
{
    if (_callback != nullptr)
	{
        _сallback(state);
	    // do something
	}
}

Причем если в качестве колбэка в SetCallback передать явно NULL, для случая с std::function ошибка обнаружится уже на этапе компиляции. А вот при отписке с использованием дефолтного конструктора boost::function() ошибка обнаруживается при исполнении кода. connection.SetCallback(boost::function<void(bool)>()) теперь не выполняет отписку, а устанавливает NULL как колбэка. В методе ConnectionChanged колбэк проходит проверку на nullptr, вызов фактически пустого колбэка приводит к исключению boost::bad_function_call. Причиной нарушения обратной совместимости стало то, что для boost::function конструктор по умолчанию возвращает NULL, а для std::function — nullptr.

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

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

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

Комментарии (6)


  1. Mingun
    18.07.2024 16:46
    +2

    Думал, будет расследование, что же происходит и почему так, а тут только факт констатируют.


    1. NickDoom
      18.07.2024 16:46
      +1

      Насколько я понял месседж, «хищник опасный, но настолько краснокнижный, что получить от него зубами — уже сенсация» :)


  1. KanuTaH
    18.07.2024 16:46
    +12

    Дело тут вообще не в сравнении NULL и nullptr. У std::function есть вот такой конструктор:

    template<class F> function( F&& f );

    Он принимает любой function object, у которого есть подходящий по сигнатуре operator(), например:

    struct Foo
    {
      void operator()(int) {}
    };
    
    void SetCallback(std::function<void(int)>) {}
    
    int main()
    {
      SetCallback(Foo()); // OK
    }

    В вашем случае роль этого Foo выполняет boost::function, ведь у нее тоже есть operator() с подходящей сигнатурой. Другое дело, что std::function в результате такой своеобразной "конверсии из boost::function" не становится "пустой", потому что откуда ее конструктор знает, что она должна таковой стать? Ему передали какой-то сторонний function object, про который он ничего по сути не знает, но который вполне подходит под требуемую сигнатуру, и при этом это не один из специальных случаев (nullptr или другая пустая std::function), поэтому проверка if (_callback != nullptr) {...} проходит (кстати, зачем так длинно, можно просто if (_callback) {...}), вызывается operator() у boost::function и выбрасывает исключение по очевидным причинам.

    connection.SetCallback(boost::function<void(bool)>()) теперь не выполняет отписку, а устанавливает NULL как колбэка

    Нет, это не так. Это не то, что здесь происходит.


  1. mentin
    18.07.2024 16:46

    К подробному описанию KanuTaH выше, я бы добавил что даже если исправить отписку, передавая nullptr вместо пустого boost::function, но оставить подписку через boost::function, то останется ухудшение производительности, если вам она важна. Теперь вместо одной обертки будет двойная, накладные расходы на вызов колбэка вырастут. В общем, лучше не смешивать разные библиотеки.


  1. boldape
    18.07.2024 16:46

    Я сегодня злой и буду грубить. Писать что

    connection.SetCallback(boost::function<void(bool)>());

    И

    connection.SetCallback(NULL);

    Одно и тоже значит расписаться в собственном непонимании концепции типов в общем и перегрузке методов в с++ в частности от слова совсем.

    boost::function конструктор по умолчанию возвращает NULL, а для std::function — nullptr.

    Бред из той же категории, говорящий о том, что такое конструктор в с++ автор не знает.

    Изначально NULL я использовала в системном программировании на Си. При переходе на C++,

    Пожалуйста, продолжайте писать на Си и больше не пытайтесь переходить на с++ пока не усвоите матчасть.

    Проблема вашего кода в том, что люди его написавшие принадлежат секте "явное лучше не явного" ну и заодно они же находят какое то мазохическое удовольствие набирать на клавиатуре бойлерплэйт. Вместо явного вызова конструктора буст фанктионал по умолчанию нужно было написать тот самый nullptr который якобы не явный и вообще не понятно чё за магия, это и сейчас все ещё нужно сделать, что бы починить все как следует. Тогда при переходе библиотеки с буста на стд ничего бы этого не случилось. А ещё лучше предоставить параметр по умолчанию что конечно противоречит религии явное лучше не явного.

    connection.SetCallback(nullptr);

    void SetCallback(std::function<...> cb = nullptr);

    connection.SetCallback();


  1. vasan
    18.07.2024 16:46

    От NULL надо отказаться раз и навсегда.