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

don't start kernel level platform timer when applying blocks #1130

Merged
merged 2 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions libraries/chain/include/eosio/chain/platform_timer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,19 @@ struct platform_timer {
platform_timer();
~platform_timer();

//start() & stop() are not thread safe to each other; i.e. do not overlap calls to start() and stop()
void start(fc::time_point tp);
void stop();
//interrupt_timer() can be called from any thread
void interrupt_timer();

/* Sets a callback for when timer expires. Be aware this could might fire from a signal handling context and/or
/* Sets a callback for when timer expires. Be aware this could fire from a signal handling context and/or
on any particular thread. Only a single callback can be registered at once; trying to register more will
result in an exception. Setting to nullptr disables any current set callback */
result in an exception. Setting to nullptr disables any current set callback.
Also, stop() is not perfectly synchronized with the callback. It is possible for stop() to return and the
callback still execute if the timer expires and stop() is called nearly simultaneously.
However, set_expiration_callback() is synchronized with the callback.
*/
void set_expiration_callback(void(*func)(void*), void* user) {
bool expect_false = false;
while(!atomic_compare_exchange_strong(&_callback_variables_busy, &expect_false, true))
Expand All @@ -45,6 +51,7 @@ struct platform_timer {
void expire_now();

std::atomic<state_t> _state = state_t::stopped;
bool timer_running_forever = false;

struct impl;
constexpr static size_t fwd_size = 8;
Expand Down
12 changes: 9 additions & 3 deletions libraries/chain/platform_timer_asio_fallback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,14 @@ platform_timer::~platform_timer() {
}

void platform_timer::start(fc::time_point tp) {
if(tp == fc::time_point::maximum()) {
assert(_state == state_t::stopped);
timer_running_forever = tp == fc::time_point::maximum();
if(timer_running_forever) {
_state = state_t::running;
return;
}
fc::microseconds x = tp.time_since_epoch() - fc::time_point::now().time_since_epoch();
timer_running_forever = false;
if(x.count() <= 0)
_state = state_t::timed_out;
else {
Expand Down Expand Up @@ -89,11 +92,14 @@ void platform_timer::interrupt_timer() {
}

void platform_timer::stop() {
if(_state == state_t::stopped)
const state_t prior_state = _state;
if(prior_state == state_t::stopped)
return;
_state = state_t::stopped;
if(prior_state == state_t::timed_out || timer_running_forever)
return;

my->timer->cancel();
_state = state_t::stopped;
}

}}
19 changes: 13 additions & 6 deletions libraries/chain/platform_timer_kqueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,20 +88,23 @@ platform_timer::~platform_timer() {
}

void platform_timer::start(fc::time_point tp) {
if(tp == fc::time_point::maximum()) {
expired = false;
assert(_state == state_t::stopped);
timer_running_forever = tp == fc::time_point::maximum();
if(timer_running_forever) {
_state = state_t::running;
return;
}
fc::microseconds x = tp.time_since_epoch() - fc::time_point::now().time_since_epoch();
timer_running_forever = false;
if(x.count() <= 0)
expired = true;
_state = state_t::timed_out;
else {
struct kevent64_s aTimerEvent;
EV_SET64(&aTimerEvent, my->timerid, EVFILT_TIMER, EV_ADD|EV_ENABLE|EV_ONESHOT, NOTE_USECONDS|NOTE_CRITICAL, x.count(), (uint64_t)this, 0, 0);

expired = false;
_state = state_t::running;
if(kevent64(kqueue_fd, &aTimerEvent, 1, NULL, 0, KEVENT_FLAG_IMMEDIATE, NULL) != 0)
expired = true;
_state = state_t::timed_out;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this wasn't updated completely in #1107. We should really refactor the code a bit to move the common state logic out of the platform specific impls.

}
}

Expand All @@ -120,7 +123,11 @@ void platform_timer::interrupt_timer() {
}

void platform_timer::stop() {
if(_state == state_t::stopped)
const state_t prior_state = _state;
if(prior_state == state_t::stopped)
return;
_state = state_t::stopped;
if(prior_state == state_t::timed_out || timer_running_forever)
return;

struct kevent64_s stop_timer_event;
Expand Down
10 changes: 7 additions & 3 deletions libraries/chain/platform_timer_posix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ platform_timer::~platform_timer() {

void platform_timer::start(fc::time_point tp) {
assert(_state == state_t::stopped);
if(tp == fc::time_point::maximum()) {
timer_running_forever = tp == fc::time_point::maximum();
if(timer_running_forever) {
_state = state_t::running;
return;
}
Expand Down Expand Up @@ -88,11 +89,14 @@ void platform_timer::interrupt_timer() {
}

void platform_timer::stop() {
if(_state == state_t::stopped)
const state_t prior_state = _state;
if(prior_state == state_t::stopped)
return;
_state = state_t::stopped;
if(prior_state == state_t::timed_out || timer_running_forever)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another minor tweak here is to not stop the timer if it has already expired. Obviously that's a rather rare scenario, but might as well be thorough while tweaking all this.

return;
struct itimerspec disable = {{0, 0}, {0, 0}};
timer_settime(my->timerid, 0, &disable, NULL);
_state = state_t::stopped;
}

}
8 changes: 6 additions & 2 deletions libraries/chain/transaction_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ namespace eosio::chain {
init_net_usage = initial_net_usage;

// set maximum to a semi-valid deadline to allow for pause math and conversion to dates for logging
if( block_deadline == fc::time_point::maximum() ) block_deadline = start + fc::hours(24*7*52);
const fc::time_point far_future_time = start + fc::days(7*52);
if( block_deadline == fc::time_point::maximum() ) block_deadline = far_future_time;

const auto& cfg = control.get_global_properties().configuration;
auto& rl = control.get_mutable_resource_limits_manager();
Expand Down Expand Up @@ -250,7 +251,10 @@ namespace eosio::chain {
_deadline = block_deadline;
}

transaction_timer.start( _deadline );
if(_deadline < far_future_time)
transaction_timer.start( _deadline );
else
transaction_timer.start( fc::time_point::maximum() ); //avoids overhead in starting the timer at all
checktime(); // Fail early if deadline has already been exceeded
is_initialized = true;
}
Expand Down
Loading