Fix linked list and shadowing bugs in kernel clock and timer code.
This commit is contained in:
parent
e962f5e4cc
commit
8ec5d9af44
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright (c) 2013 Jonas 'Sortie' Termansen.
|
* Copyright (c) 2013, 2016 Jonas 'Sortie' Termansen.
|
||||||
*
|
*
|
||||||
* Permission to use, copy, modify, and distribute this software for any
|
* Permission to use, copy, modify, and distribute this software for any
|
||||||
* purpose with or without fee is hereby granted, provided that the above
|
* purpose with or without fee is hereby granted, provided that the above
|
||||||
|
@ -136,14 +136,14 @@ void Clock::RegisterAbsolute(Timer* timer) // Lock acquired.
|
||||||
timer->flags |= TIMER_ACTIVE;
|
timer->flags |= TIMER_ACTIVE;
|
||||||
|
|
||||||
Timer* before = NULL;
|
Timer* before = NULL;
|
||||||
for ( Timer* iter = absolute_timer; iter; iter = before->next_timer )
|
for ( Timer* iter = absolute_timer; iter; iter = iter->next_timer )
|
||||||
|
{
|
||||||
if ( timespec_lt(timer->value.it_value, iter->value.it_value) )
|
if ( timespec_lt(timer->value.it_value, iter->value.it_value) )
|
||||||
break;
|
|
||||||
else
|
|
||||||
before = iter;
|
before = iter;
|
||||||
|
}
|
||||||
|
|
||||||
timer->prev_timer = before;
|
timer->prev_timer = before;
|
||||||
timer->next_timer = before ? before->next_timer : NULL;
|
timer->next_timer = before ? before->next_timer : absolute_timer;
|
||||||
if ( timer->next_timer ) timer->next_timer->prev_timer = timer;
|
if ( timer->next_timer ) timer->next_timer->prev_timer = timer;
|
||||||
(before ? before->next_timer : absolute_timer) = timer;
|
(before ? before->next_timer : absolute_timer) = timer;
|
||||||
}
|
}
|
||||||
|
@ -154,19 +154,22 @@ void Clock::RegisterDelay(Timer* timer) // Lock acquired.
|
||||||
timer->flags |= TIMER_ACTIVE;
|
timer->flags |= TIMER_ACTIVE;
|
||||||
|
|
||||||
Timer* before = NULL;
|
Timer* before = NULL;
|
||||||
struct timespec before_time = timespec_nul();
|
for ( Timer* iter = delay_timer; iter; iter = iter->next_timer )
|
||||||
for ( Timer* iter = delay_timer; iter; iter = before->next_timer )
|
{
|
||||||
if ( timespec_lt(timer->value.it_value, iter->value.it_value) )
|
if ( timespec_lt(timer->value.it_value, iter->value.it_value) )
|
||||||
break;
|
break;
|
||||||
else
|
timer->value.it_value = timespec_sub(timer->value.it_value, iter->value.it_value);
|
||||||
before = iter,
|
before = iter;
|
||||||
before_time = timespec_add(before_time, iter->value.it_value);
|
}
|
||||||
|
|
||||||
timer->value.it_value = timespec_sub(timer->value.it_value, before_time);
|
|
||||||
timer->prev_timer = before;
|
timer->prev_timer = before;
|
||||||
timer->next_timer = before ? before->next_timer : NULL;
|
timer->next_timer = before ? before->next_timer : delay_timer;
|
||||||
if ( timer->next_timer ) timer->next_timer->prev_timer = timer;
|
if ( timer->next_timer )
|
||||||
(before ? before->next_timer : delay_timer) = timer;
|
timer->next_timer->prev_timer = timer;
|
||||||
|
if ( before )
|
||||||
|
before->next_timer = timer;
|
||||||
|
else
|
||||||
|
delay_timer = timer;
|
||||||
|
|
||||||
if ( timer->next_timer )
|
if ( timer->next_timer )
|
||||||
timer->next_timer->value.it_value =
|
timer->next_timer->value.it_value =
|
||||||
|
@ -183,6 +186,7 @@ void Clock::Register(Timer* timer)
|
||||||
|
|
||||||
void Clock::UnlinkAbsolute(Timer* timer) // Lock acquired.
|
void Clock::UnlinkAbsolute(Timer* timer) // Lock acquired.
|
||||||
{
|
{
|
||||||
|
assert(timer->flags & TIMER_ACTIVE);
|
||||||
(timer->prev_timer ? timer->prev_timer->next_timer : absolute_timer) = timer->next_timer;
|
(timer->prev_timer ? timer->prev_timer->next_timer : absolute_timer) = timer->next_timer;
|
||||||
if ( timer->next_timer ) timer->next_timer->prev_timer = timer->prev_timer;
|
if ( timer->next_timer ) timer->next_timer->prev_timer = timer->prev_timer;
|
||||||
timer->prev_timer = timer->next_timer = NULL;
|
timer->prev_timer = timer->next_timer = NULL;
|
||||||
|
@ -191,6 +195,7 @@ void Clock::UnlinkAbsolute(Timer* timer) // Lock acquired.
|
||||||
|
|
||||||
void Clock::UnlinkDelay(Timer* timer) // Lock acquired.
|
void Clock::UnlinkDelay(Timer* timer) // Lock acquired.
|
||||||
{
|
{
|
||||||
|
assert(timer->flags & TIMER_ACTIVE);
|
||||||
(timer->prev_timer ? timer->prev_timer->next_timer : delay_timer) = timer->next_timer;
|
(timer->prev_timer ? timer->prev_timer->next_timer : delay_timer) = timer->next_timer;
|
||||||
if ( timer->next_timer ) timer->next_timer->prev_timer = timer->prev_timer;
|
if ( timer->next_timer ) timer->next_timer->prev_timer = timer->prev_timer;
|
||||||
if ( timer->next_timer ) timer->next_timer->value.it_value = timespec_add(timer->next_timer->value.it_value, timer->value.it_value);
|
if ( timer->next_timer ) timer->next_timer->value.it_value = timespec_add(timer->next_timer->value.it_value, timer->value.it_value);
|
||||||
|
@ -251,8 +256,10 @@ struct timespec Clock::SleepDelay(struct timespec duration)
|
||||||
LockClock();
|
LockClock();
|
||||||
|
|
||||||
if ( !start_advancement_set )
|
if ( !start_advancement_set )
|
||||||
start_advancement = current_advancement,
|
{
|
||||||
|
start_advancement = current_advancement;
|
||||||
start_advancement_set = true;
|
start_advancement_set = true;
|
||||||
|
}
|
||||||
|
|
||||||
elapsed = timespec_sub(current_advancement, start_advancement);
|
elapsed = timespec_sub(current_advancement, start_advancement);
|
||||||
|
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright (c) 2013 Jonas 'Sortie' Termansen.
|
* Copyright (c) 2013, 2016 Jonas 'Sortie' Termansen.
|
||||||
*
|
*
|
||||||
* Permission to use, copy, modify, and distribute this software for any
|
* Permission to use, copy, modify, and distribute this software for any
|
||||||
* purpose with or without fee is hereby granted, provided that the above
|
* purpose with or without fee is hereby granted, provided that the above
|
||||||
|
@ -30,11 +30,11 @@ namespace Sortix {
|
||||||
class Clock;
|
class Clock;
|
||||||
class Timer;
|
class Timer;
|
||||||
|
|
||||||
const int TIMER_ABSOLUTE = 1 << 0;
|
static const int TIMER_ABSOLUTE = 1 << 0;
|
||||||
const int TIMER_ACTIVE = 1 << 1;
|
static const int TIMER_ACTIVE = 1 << 1;
|
||||||
const int TIMER_FIRING = 1 << 2;
|
static const int TIMER_FIRING = 1 << 2;
|
||||||
const int TIMER_FUNC_INTERRUPT_HANDLER = 1 << 3;
|
static const int TIMER_FUNC_INTERRUPT_HANDLER = 1 << 3;
|
||||||
const int TIMER_FUNC_ADVANCE_THREAD = 1 << 4;
|
static const int TIMER_FUNC_ADVANCE_THREAD = 1 << 4;
|
||||||
|
|
||||||
class Timer
|
class Timer
|
||||||
{
|
{
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright (c) 2013 Jonas 'Sortie' Termansen.
|
* Copyright (c) 2013, 2016 Jonas 'Sortie' Termansen.
|
||||||
*
|
*
|
||||||
* Permission to use, copy, modify, and distribute this software for any
|
* Permission to use, copy, modify, and distribute this software for any
|
||||||
* purpose with or without fee is hereby granted, provided that the above
|
* purpose with or without fee is hereby granted, provided that the above
|
||||||
|
@ -98,8 +98,9 @@ void Timer::Get(struct itimerspec* current)
|
||||||
clock->UnlockClock();
|
clock->UnlockClock();
|
||||||
}
|
}
|
||||||
|
|
||||||
void Timer::Set(struct itimerspec* value, struct itimerspec* ovalue, int flags,
|
void Timer::Set(struct itimerspec* new_value, struct itimerspec* old_value,
|
||||||
void (*callback)(Clock*, Timer*, void*), void* user)
|
int new_flags, void (*new_callback)(Clock*, Timer*, void*),
|
||||||
|
void* new_user)
|
||||||
{
|
{
|
||||||
assert(clock);
|
assert(clock);
|
||||||
|
|
||||||
|
@ -107,19 +108,23 @@ void Timer::Set(struct itimerspec* value, struct itimerspec* ovalue, int flags,
|
||||||
|
|
||||||
// Dequeue this timer if it is already armed.
|
// Dequeue this timer if it is already armed.
|
||||||
if ( flags & TIMER_ACTIVE )
|
if ( flags & TIMER_ACTIVE )
|
||||||
|
{
|
||||||
|
// TODO: How does this interplay with concurrently running timer
|
||||||
|
// handlers? Maybe the timer should be cancelled instead?
|
||||||
clock->Unlink(this);
|
clock->Unlink(this);
|
||||||
|
}
|
||||||
|
|
||||||
// Let the caller know how much time was left on the timer.
|
// Let the caller know how much time was left on the timer.
|
||||||
if ( ovalue )
|
if ( old_value )
|
||||||
GetInternal(ovalue);
|
GetInternal(old_value);
|
||||||
|
|
||||||
// Arm the timer if a value was specified.
|
// Arm the timer if a value was specified.
|
||||||
if ( timespec_lt(timespec_nul(), value->it_value) )
|
if ( timespec_lt(timespec_nul(), new_value->it_value) )
|
||||||
{
|
{
|
||||||
this->value = *value;
|
value = *new_value;
|
||||||
this->flags = flags;
|
flags = new_flags;
|
||||||
this->callback = callback;
|
callback = new_callback;
|
||||||
this->user = user;
|
user = new_user;
|
||||||
clock->Register(this);
|
clock->Register(this);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue