Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop custom implementation of boost::condition_variable to fix busy-wait spinning #1878

Merged
merged 13 commits into from
Feb 8, 2020

Commits on Feb 5, 2020

  1. roscpp: fix potential busy-wait loop caused by backported Boost condi…

    …tion_variable (fix ros#1343)
    
    ros#1014 and ros#1250 introduced a backported
    version of boost::condition_variable, where support for steady (monotonic) clocks has been added in version 1.61.
    But the namespace of the backported version was not changed and the symbol names might clash with the original
    version.
    
    Because the underlying clock used for the condition_variable is set in the constructor and must be
    consistent with the the expectations within member variables. The compiler might choose to inline one or the
    other or both, and is more likely to do so for optimized Release builds. But if it does not, the symbol ends
    up in the symbol table of roscpp and depending on which other libraries will be linked into the process it
    is unpredictable which of the two versions will be actually called at the end. In case the constructor defined
    in `/usr/include/boost/thread/pthread/condition_variable.hpp` was called and did not configure the internal
    pthread condition variable for monotonic clock, each call to the backported do_wait_until() method with a
    monotonic timestamp will return immediately and hence causes `CallbackQueue::callOne(timeout)` or
    `CallbackQueue::callAvailable(timeout)` to return immediately.
    
    This patch changes the namespace of the backported condition_variable implementation to boost_161. This
    removes the ambiguity with the original definition if both are used in the same process.
    meyerj authored and dirk-thomas committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    2577130 View commit details
    Browse the repository at this point in the history
  2. roscpp: use boost::condition_variable::wait_for() instead of deprecat…

    …ed timed_wait()
    
    This fixes ROS timers in combination with 2c18b9f. The timer
    callbacks were not called because the TimerManager's thread function blocked indefinitely on
    boost::condition_variable::timed_wait().
    
    Relative timed_wait() uses the system clock (boost::get_system_time()) unconditionally to
    calculate the absolute timestamp for do_wait_until(). If the condition variable has been
    initialized with BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC, it compares this timestamp
    with the monotonic clock and therefore blocks.
    
    This issue has been reported in https://svn.boost.org/trac10/ticket/12728 and will not be
    fixed. The timed_wait interface is apparently deprecated.
    meyerj authored and dirk-thomas committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    b4e7cbc View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    be288e2 View commit details
    Browse the repository at this point in the history
  4. roscpp: remove specialized implementation of TimerManager<T,D,E>::thr…

    …eadFunc() in steady_timer.cpp
    
    The updated generic definition in timer_manager.h should do the same with a minor update.
    In all cases we can call boost::condition_variable::wait_until() with an absolute time_point of the respective clock.
    The conversion from system_clock to steady_clock for Time and WallTime is done internally in
    boost::condition_variable::wait_until(lock_type& lock, const chrono::time_point<Clock, Duration>& t).
    meyerj authored and dirk-thomas committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    9d2715d View commit details
    Browse the repository at this point in the history
  5. fix namespaces

    Antoine Hoarau authored and dirk-thomas committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    f6a05e3 View commit details
    Browse the repository at this point in the history
  6. add more explicit namespaces

    ahoarau authored and dirk-thomas committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    dd503e2 View commit details
    Browse the repository at this point in the history
  7. add missing ns

    Antoine Hoarau authored and dirk-thomas committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    48b3f6b View commit details
    Browse the repository at this point in the history
  8. roscpp: fixed Boost version check in CMakeLists.txt

    find_package(Boost) has to come before checking the Boost version.
    Otherwise BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC was not defined which
    triggered the assertion in timer_manager.h:240.
    
    Since Boost 1.67 BOOST_THREAD_HAS_CONDATTR_SET_CLOCK_MONOTONIC became the default
    if the platform supports it and the macro is not defined anymore. Instead, check
    for BOOST_THREAD_INTERNAL_CLOCK_IS_MONO.
    meyerj authored and dirk-thomas committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    dfbf726 View commit details
    Browse the repository at this point in the history
  9. roscpp: replace ROSCPP_BOOST_CONDITION_VARIABLE and ROSCPP_BOOST_COND…

    …ITION_VARIABLE_HEADER macros by a typedef in internal_condition_variable.h
    meyerj authored and dirk-thomas committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    b9e8f3a View commit details
    Browse the repository at this point in the history
  10. Remove copy of boost::condition_variable implementation from Boost 1.…

    …61 in namespace boost_161
    meyerj committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    8e44572 View commit details
    Browse the repository at this point in the history
  11. Revert some changes in include directives and in CMakeLists.txt to mi…

    …nimize the diff to melodic-devel
    
    Addresses ros#1878 (review).
    meyerj committed Feb 5, 2020
    Configuration menu
    Copy the full SHA
    7d470f3 View commit details
    Browse the repository at this point in the history

Commits on Feb 6, 2020

  1. 4 Configuration menu
    Copy the full SHA
    2a67cf6 View commit details
    Browse the repository at this point in the history

Commits on Feb 7, 2020

  1. Revert "use wait_for(), remove TimerManagerTraits"

    This reverts commit 2a67cf6.
    dirk-thomas committed Feb 7, 2020
    Configuration menu
    Copy the full SHA
    5d72aa2 View commit details
    Browse the repository at this point in the history