Skip to content

Commit

Permalink
Consistently use a monotonic clock source for the gnulinux target
Browse files Browse the repository at this point in the history 
The absolute time returned by rtos_get_time_ns() or RTT::os::TimeService::getNSecs() was generated from
a wall clock (time of day), which is not monotonic and can jump in certain cases, e.g. due to an NTP update.
This time is used in several internal and public API functions to implement timers or to wait on a
condition variable, mutex or semaphore with a timeout. In all these cases non-steady system time can have
undesirable side effects, which are not expected in a Real-Time Toolkit.

There have been several attempts to fix or at least mitigate that problem:
- http://bugs.orocos.org/show_bug.cgi?id=735
- orocos-toolchain#129
- orocos-toolchain#138

This patch introduces some potentially breaking changes:
- Use clock_gettime(CLOCK_MONOTONIC, ...) to implement rtos_get_time_ns() for the gnulinux target.
  This change has a major impact on all usages of that method, rtos_get_ticks() or the higher-level
  methods RTT::os::TimeService::getNSecs() and RTT::os::TimeService::getTicks(), whose return values
  cannot be interpreted as wall time anymore.
- Consequently remove the rtos_get_time_monotonic-ns() function introduced in
  orocos-toolchain#138.
- Drop RTT::os::Semaphore::waitUntil() and the underlying OS abstraction functions.
  The only usage within the RTT core was in RTT::os::Timer, which can easily be rewritten using
  condition variables. POSIX semaphores do not support waiting with an absolute timeout from the
  monotonic clock.
- Added new OS abstraction functions for waiting on a mutex lock with a relative timeout. POSIX does
  not provide a pthread_mutexattr_setclock() method similar to pthread_condattr_setclock(). For that
  reason rtos_mutex_lock_until() and rtos_mutex_trylock_for() are currently falling back to
  CLOCK_REALTIME.

Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
  • Loading branch information
@meyerj
meyerj committed Mar 1, 2018
1 parent 7aa885e commit 1c0ddce
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 154 deletions.
4 changes: 2 additions & 2 deletions rtt/os/Mutex.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ namespace RTT
*/
virtual bool timedlock(Seconds s)
{
if ( rtos_mutex_lock_until( &m, rtos_get_time_ns() + Seconds_to_nsecs(s) ) == 0 )
if ( rtos_mutex_trylock_for( &m, Seconds_to_nsecs(s) ) == 0 )
return true;
return false;
}
Expand Down Expand Up @@ -263,7 +263,7 @@ namespace RTT
*/
virtual bool timedlock(Seconds s)
{
if ( rtos_mutex_rec_lock_until( &recm, rtos_get_time_ns() + Seconds_to_nsecs(s) ) == 0 )
if ( rtos_mutex_rec_trylock_for( &recm, Seconds_to_nsecs(s) ) == 0 )
return true;
return false;
}
Expand Down
28 changes: 0 additions & 28 deletions rtt/os/Semaphore.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,34 +110,6 @@ namespace RTT
return false;
}

/**
* Wait on this semaphore until a maximum absolute time.
* @param abs_time Absolute time in seconds until which to wait on this semaphore.
* @retval true if the semaphore was signaled before abs_time expired.
* @retval false if the semaphore was not signaled.
* @see rtos_get_time_ns() to get the current time in nano seconds.
*/
bool waitUntil( Seconds abs_time )
{
if (rtos_sem_wait_until( &sem, Seconds_to_nsecs(abs_time) ) == 0 )
return true;
return false;
}

/**
* Wait on this semaphore until a maximum absolute time.
* @param abs_time Absolute time in nano seconds until which to wait on this semaphore.
* @retval true if the semaphore was signaled before abs_time expired.
* @retval false if the semaphore was not signaled.
* @see rtos_get_time_ns() to get the current time in nano seconds.
*/
bool waitUntil( nsecs abs_time )
{
if (rtos_sem_wait_until( &sem, abs_time ) == 0 )
return true;
return false;
}

/**
* Return the current count of this semaphore.
*/
Expand Down
57 changes: 27 additions & 30 deletions rtt/os/Timer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,34 +66,34 @@ namespace RTT {
TimerId next_timer_id = 0;

// Select next timer.
int ret = 0;
{// This scope is for MutexLock.
// find wake_up_time
// check timers queue.
MutexLock locker(m);
// We can't use infinite as the OS may internally use time_spec, which can not
// represent as much in the future (until 2038) // XXX Year-2038 Bug
wake_up_time = 1000000000LL * std::numeric_limits<int32_t>::max();
MutexLock locker(mmutex);
wake_up_time = std::numeric_limits<Time>::max();
for (TimerIds::iterator it = mtimers.begin(); it != mtimers.end(); ++it) {
if ( it->expires != 0 && it->expires < wake_up_time ) {
wake_up_time = it->expires;
next_timer_id = it - mtimers.begin();
}
}
}// MutexLock

// Wait
int ret = 0;
if ( wake_up_time > rtos_get_time_ns() )
ret = msem.waitUntil( wake_up_time ); // case of no timers or running timers
else
ret = -1; // case of timer overrun.
// Wait
if ( wake_up_time == std::numeric_limits<Time>::max() )
ret = mcond.wait( mmutex ); // case of no timers
else if ( wake_up_time > rtos_get_time_ns() )
ret = mcond.wait_until( mmutex, wake_up_time ); // case of running timers
else
ret = -1; // case of timer overrun.
}// MutexLock

// Timeout handling
if (ret == -1) {
// a timer expired
// expires: reset/reprogram the timer that expired:
{
MutexLock locker(m);
MutexLock locker(mmutex);
// detect corner case for resize:
if ( next_timer_id < int(mtimers.size()) ) {
// now clear or reprogram it.
Expand All @@ -110,7 +110,7 @@ namespace RTT {

// Second: notify waiting threads
{// This scope is for MutexLock.
MutexLock locker(m);
MutexLock locker(mmutex);
mtimers[next_timer_id].expired.broadcast();
}// MutexLock

Expand All @@ -126,7 +126,7 @@ namespace RTT {
bool Timer::breakLoop()
{
mdo_quit = true;
msem.signal();
mcond.broadcast();
// kill all timers to abort all threads blocking in waitFor()
for (TimerId i = 0; i < (int) mtimers.size(); ++i) {
killTimer(i);
Expand All @@ -135,7 +135,7 @@ namespace RTT {
}

Timer::Timer(TimerId max_timers, int scheduler, int priority)
: mThread(0), msem(0), mdo_quit(false)
: mThread(0), mdo_quit(false)
{
mtimers.resize(max_timers);
if (scheduler != -1) {
Expand All @@ -157,7 +157,7 @@ namespace RTT {

void Timer::setMaxTimers(TimerId max)
{
MutexLock locker(m);
MutexLock locker(mmutex);
mtimers.resize(max, TimerInfo() );
}

Expand All @@ -172,11 +172,11 @@ namespace RTT {
Time due_time = rtos_get_time_ns() + Seconds_to_nsecs( period );

{
MutexLock locker(m);
MutexLock locker(mmutex);
mtimers[timer_id].expires = due_time;
mtimers[timer_id].period = Seconds_to_nsecs( period );
}
msem.signal();
mcond.broadcast();
return true;
}

Expand All @@ -192,17 +192,17 @@ namespace RTT {
Time due_time = now + Seconds_to_nsecs( wait_time );

{
MutexLock locker(m);
MutexLock locker(mmutex);
mtimers[timer_id].expires = due_time;
mtimers[timer_id].period = 0;
}
msem.signal();
mcond.broadcast();
return true;
}

bool Timer::isArmed(TimerId timer_id) const
{
MutexLock locker(m);
MutexLock locker(mmutex);
if (timer_id < 0 || timer_id >= int(mtimers.size()) )
{
log(Error) << "Invalid timer id" << endlog();
Expand All @@ -213,7 +213,7 @@ namespace RTT {

double Timer::timeRemaining(TimerId timer_id) const
{
MutexLock locker(m);
MutexLock locker(mmutex);
if (timer_id < 0 || timer_id >= int(mtimers.size()) )
{
log(Error) << "Invalid timer id" << endlog();
Expand All @@ -229,7 +229,7 @@ namespace RTT {

bool Timer::killTimer(TimerId timer_id)
{
MutexLock locker(m);
MutexLock locker(mmutex);
if (timer_id < 0 || timer_id >= int(mtimers.size()) )
{
log(Error) << "Invalid timer id" << endlog();
Expand All @@ -243,30 +243,27 @@ namespace RTT {

bool Timer::waitFor(TimerId timer_id)
{
MutexLock locker(m);
MutexLock locker(mmutex);
if (timer_id < 0 || timer_id >= int(mtimers.size()) )
{
log(Error) << "Invalid timer id" << endlog();
return false;
}
if (mtimers[timer_id].expires == 0) return false;

return mtimers[timer_id].expired.wait(m);
return mtimers[timer_id].expired.wait(mmutex);
}

bool Timer::waitForUntil(TimerId timer_id, nsecs abs_time)
{
MutexLock locker(m);
MutexLock locker(mmutex);
if (timer_id < 0 || timer_id >= int(mtimers.size()) )
{
log(Error) << "Invalid timer id" << endlog();
return false;
}
if (mtimers[timer_id].expires == 0) return false;

return mtimers[timer_id].expired.wait_until(m, abs_time);
return mtimers[timer_id].expired.wait_until(mmutex, abs_time);
}



}
4 changes: 2 additions & 2 deletions rtt/os/Timer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ namespace RTT
typedef int TimerId;
protected:
base::ActivityInterface* mThread;
Semaphore msem;
mutable Mutex m;
Condition mcond;
mutable Mutex mmutex;
typedef TimeService::nsecs Time;

struct TimerInfo
Expand Down
6 changes: 3 additions & 3 deletions rtt/os/fosi_interface.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ extern "C"
int rtos_sem_signal(rt_sem_t* m );
int rtos_sem_wait(rt_sem_t* m );
int rtos_sem_trywait(rt_sem_t* m );
int rtos_sem_wait_timed(rt_sem_t* m, NANO_TIME delay );
int rtos_sem_wait_until(rt_sem_t* m, NANO_TIME abs_time );
int rtos_sem_value(rt_sem_t* m );

// Mutex and recursive mutex functions
Expand All @@ -117,10 +115,12 @@ extern "C"
int rtos_mutex_rec_destroy(rt_rec_mutex_t* m );
int rtos_mutex_lock( rt_mutex_t* m);
int rtos_mutex_trylock( rt_mutex_t* m);
int rtos_mutex_lock_until( rt_rec_mutex_t* m, NANO_TIME abs_time);
int rtos_mutex_trylock_for( rt_mutex_t* m, NANO_TIME relative_time);
int rtos_mutex_lock_until( rt_mutex_t* m, NANO_TIME abs_time);
int rtos_mutex_unlock( rt_mutex_t* m);
int rtos_mutex_rec_lock( rt_rec_mutex_t* m);
int rtos_mutex_rec_trylock( rt_rec_mutex_t* m);
int rtos_mutex_rec_trylock_for( rt_rec_mutex_t* m, NANO_TIME relative_time);
int rtos_mutex_rec_lock_until( rt_rec_mutex_t* m, NANO_TIME abs_time);
int rtos_mutex_rec_unlock( rt_rec_mutex_t* m);

Expand Down

0 comments on commit 1c0ddce

Please sign in to comment.