Skip to content

Commit 6e46925

Browse files
spoonincodespoonincode
authored
Merge pull request #1820 from AntelopeIO/multitimer_aba_fix_ptrtag_12x
[1.2.2] protect against timer "rearm race": more timers approach with pointer tagging
2 parents 19e7047 + d516c58 commit 6e46925

File tree

4 files changed

+103
-73
lines changed

4 files changed

+103
-73
lines changed

‎libraries/chain/include/eosio/chain/platform_timer.hpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,21 @@ struct platform_timer {
4848
state_t timer_state() const { return _state.load().state; }
4949

5050
private:
51-
void expire_now();
51+
using generation_t = uint16_t;
52+
53+
void expire_now(generation_t expired_generation);
5254

5355
struct timer_state_t {
5456
state_t state = state_t::stopped;
5557
bool callback_in_flight = false;
58+
generation_t generation_running = 0;
5659
};
5760
std::atomic<timer_state_t> _state;
5861
bool timer_running_forever = false;
62+
generation_t generation = 0;
5963

6064
struct impl;
61-
constexpr static size_t fwd_size = 8;
65+
constexpr static size_t fwd_size = 64;
6266
fc::fwd<impl,fwd_size> my;
6367

6468
void call_expiration_callback() {

‎libraries/chain/platform_timer_asio_fallback.cpp‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,55 +59,57 @@ platform_timer::~platform_timer() {
5959

6060
void platform_timer::start(fc::time_point tp) {
6161
assert(timer_state() == state_t::stopped);
62+
++generation;
6263
timer_running_forever = tp == fc::time_point::maximum();
6364
if(timer_running_forever) {
64-
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false});
65+
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false, .generation_running = generation});
6566
return;
6667
}
6768
fc::microseconds x = tp.time_since_epoch() - fc::time_point::now().time_since_epoch();
6869
timer_running_forever = false;
6970
if(x.count() <= 0) {
70-
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false});
71+
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false, .generation_running = generation});
7172
} else {
72-
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false});
73+
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false, .generation_running = generation});
7374
my->timer->expires_after(std::chrono::microseconds(x.count()));
74-
my->timer->async_wait([this](const boost::system::error_code& ec) {
75+
my->timer->async_wait([this, generation=generation](const boost::system::error_code& ec) {
7576
if(ec)
7677
return;
77-
expire_now();
78+
expire_now(generation);
7879
});
7980
}
8081
}
8182

82-
void platform_timer::expire_now() {
83-
timer_state_t expected{.state = state_t::running, .callback_in_flight = false};
84-
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::timed_out, true})) {
83+
void platform_timer::expire_now(generation_t expired_generation) {
84+
timer_state_t expected{.state = state_t::running, .callback_in_flight = false, .generation_running = expired_generation};
85+
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::timed_out, true, expired_generation})) {
8586
call_expiration_callback();
86-
_state.store(timer_state_t{state_t::timed_out, false});
87+
_state.store(timer_state_t{state_t::timed_out, false, expired_generation});
8788
}
8889
}
8990

9091
void platform_timer::interrupt_timer() {
91-
timer_state_t expected{.state = state_t::running, .callback_in_flight = false};
92-
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::interrupted, true})) {
92+
const generation_t generation_running = _state.load().generation_running;
93+
timer_state_t expected{.state = state_t::running, .callback_in_flight = false, .generation_running = generation_running};
94+
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::interrupted, true, generation_running})) {
9395
call_expiration_callback();
94-
_state.store(timer_state_t{state_t::interrupted, false});
96+
_state.store(timer_state_t{state_t::interrupted, false, generation_running});
9597
}
9698
}
9799

98100
void platform_timer::stop() {
99101
// if still running, then interrupt so expire_now() and interrupt_timer() can't start a callback call
100-
timer_state_t prior_state{.state = state_t::running, .callback_in_flight = false};
101-
if (_state.compare_exchange_strong(prior_state, timer_state_t{state_t::interrupted, false})) {
102-
prior_state = timer_state_t{state_t::interrupted, false};
102+
timer_state_t prior_state{.state = state_t::running, .callback_in_flight = false, .generation_running = generation};
103+
if (_state.compare_exchange_strong(prior_state, timer_state_t{state_t::interrupted, false, generation})) {
104+
prior_state = timer_state_t{state_t::interrupted, false, generation};
103105
}
104106

105107
for (; prior_state.callback_in_flight; prior_state = _state.load())
106108
boost::core::sp_thread_pause();
107109

108110
if(prior_state.state == state_t::stopped)
109111
return;
110-
_state.store(timer_state_t{.state = state_t::stopped, .callback_in_flight = false});
112+
_state.store(timer_state_t{.state = state_t::stopped, .callback_in_flight = false, .generation_running = generation});
111113
if(prior_state.state == state_t::timed_out || timer_running_forever)
112114
return;
113115

‎libraries/chain/platform_timer_kqueue.cpp‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ static std::thread kevent_thread;
3333

3434
struct platform_timer::impl {
3535
uint64_t timerid;
36+
constexpr static unsigned tag_ptr_shift = 57; //safe for x64 w/ 5-level paging; RISC-V w/ Sv57; POWER10; ARM8.2's LVA is only 52
37+
constexpr static uint64_t tag_ptr_mask = -1ull << tag_ptr_shift;
38+
constexpr static unsigned tag_modulo = 1<<(std::numeric_limits<uint64_t>::digits - tag_ptr_shift);
3639

3740
constexpr static uint64_t quit_event_id = 1;
3841
};
@@ -59,8 +62,9 @@ platform_timer::platform_timer() {
5962
int c = kevent64(kqueue_fd, NULL, 0, &anEvent, 1, 0, NULL);
6063

6164
if(c == 1 && anEvent.filter == EVFILT_TIMER) {
62-
platform_timer* self = (platform_timer*)anEvent.udata;
63-
self->expire_now();
65+
const generation_t expiry_gen = anEvent.udata >> impl::tag_ptr_shift;
66+
platform_timer* self = reinterpret_cast<platform_timer*>(anEvent.udata & ~impl::tag_ptr_mask);
67+
self->expire_now(expiry_gen);
6468
}
6569
else if(c == 1 && anEvent.filter == EVFILT_USER)
6670
return;
@@ -91,54 +95,57 @@ platform_timer::~platform_timer() {
9195

9296
void platform_timer::start(fc::time_point tp) {
9397
assert(timer_state() == state_t::stopped);
98+
generation = (generation + 1) % impl::tag_modulo;
9499
timer_running_forever = tp == fc::time_point::maximum();
95100
if(timer_running_forever) {
96-
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false});
101+
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false, .generation_running = generation});
97102
return;
98103
}
99104
fc::microseconds x = tp.time_since_epoch() - fc::time_point::now().time_since_epoch();
100105
timer_running_forever = false;
101106
if(x.count() <= 0) {
102-
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false});
107+
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false, .generation_running = generation});
103108
} else {
104109
struct kevent64_s aTimerEvent;
105-
EV_SET64(&aTimerEvent, my->timerid, EVFILT_TIMER, EV_ADD|EV_ENABLE|EV_ONESHOT, NOTE_USECONDS|NOTE_CRITICAL, x.count(), (uint64_t)this, 0, 0);
110+
uintptr_t tagged_this = reinterpret_cast<uintptr_t>(this) | static_cast<uintptr_t>(generation)<<impl::tag_ptr_shift;
111+
EV_SET64(&aTimerEvent, my->timerid, EVFILT_TIMER, EV_ADD|EV_ENABLE|EV_ONESHOT, NOTE_USECONDS|NOTE_CRITICAL, x.count(), tagged_this, 0, 0);
106112

107-
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false});
113+
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false, .generation_running = generation});
108114
if(kevent64(kqueue_fd, &aTimerEvent, 1, NULL, 0, KEVENT_FLAG_IMMEDIATE, NULL) != 0)
109-
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false});
115+
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false, .generation_running = generation});
110116
}
111117
}
112118

113-
void platform_timer::expire_now() {
114-
timer_state_t expected{.state = state_t::running, .callback_in_flight = false};
115-
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::timed_out, true})) {
119+
void platform_timer::expire_now(generation_t expired_generation) {
120+
timer_state_t expected{.state = state_t::running, .callback_in_flight = false, .generation_running = expired_generation};
121+
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::timed_out, true, expired_generation})) {
116122
call_expiration_callback();
117-
_state.store(timer_state_t{state_t::timed_out, false});
123+
_state.store(timer_state_t{state_t::timed_out, false, expired_generation});
118124
}
119125
}
120126

121127
void platform_timer::interrupt_timer() {
122-
timer_state_t expected{.state = state_t::running, .callback_in_flight = false};
123-
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::interrupted, true})) {
128+
const generation_t generation_running = _state.load().generation_running;
129+
timer_state_t expected{.state = state_t::running, .callback_in_flight = false, .generation_running = generation_running};
130+
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::interrupted, true, generation_running})) {
124131
call_expiration_callback();
125-
_state.store(timer_state_t{state_t::interrupted, false});
132+
_state.store(timer_state_t{state_t::interrupted, false, generation_running});
126133
}
127134
}
128135

129136
void platform_timer::stop() {
130137
// if still running, then interrupt so expire_now() and interrupt_timer() can't start a callback call
131-
timer_state_t prior_state{.state = state_t::running, .callback_in_flight = false};
132-
if (_state.compare_exchange_strong(prior_state, timer_state_t{state_t::interrupted, false})) {
133-
prior_state = timer_state_t{state_t::interrupted, false};
138+
timer_state_t prior_state{.state = state_t::running, .callback_in_flight = false, .generation_running = generation};
139+
if (_state.compare_exchange_strong(prior_state, timer_state_t{state_t::interrupted, false, generation})) {
140+
prior_state = timer_state_t{state_t::interrupted, false, generation};
134141
}
135142

136143
for (; prior_state.callback_in_flight; prior_state = _state.load())
137144
boost::core::sp_thread_pause();
138145

139146
if(prior_state.state == state_t::stopped)
140147
return;
141-
_state.store(timer_state_t{.state = state_t::stopped, .callback_in_flight = false});
148+
_state.store(timer_state_t{.state = state_t::stopped, .callback_in_flight = false, .generation_running = generation});
142149
if(prior_state.state == state_t::timed_out || timer_running_forever)
143150
return;
144151

‎libraries/chain/platform_timer_posix.cpp‎

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -19,98 +19,115 @@ namespace eosio::chain {
1919
static_assert(std::atomic_bool::is_always_lock_free, "Only lock-free atomics AS-safe.");
2020

2121
struct platform_timer::impl {
22-
timer_t timerid;
22+
constexpr static unsigned num_timers = 8;
23+
static_assert(std::has_single_bit(num_timers), "num_timers must be a power of two");
24+
25+
constexpr static unsigned tag_ptr_shift = 57; //safe for x64 w/ 5-level paging; RISC-V w/ Sv57; POWER10; ARM8.2's LVA is only 52
26+
constexpr static uint64_t tag_ptr_mask = -1ull << tag_ptr_shift;
27+
28+
static_assert(std::cmp_less_equal(std::popcount(num_timers-1), sizeof(uintptr_t)*8-tag_ptr_shift), "generation count won't fit in pointer tag");
29+
30+
std::array<timer_t, num_timers> timerid;
31+
32+
struct signal_initer {
33+
signal_initer() {
34+
struct sigaction act;
35+
sigemptyset(&act.sa_mask);
36+
act.sa_sigaction = impl::sig_handler;
37+
act.sa_flags = SA_SIGINFO | SA_RESTART;
38+
FC_ASSERT(sigaction(SIGRTMIN, &act, NULL) == 0, "failed to acquire SIGRTMIN signal");
39+
}
40+
};
2341

2442
static void sig_handler(int, siginfo_t* si, void*) {
25-
platform_timer* self = (platform_timer*)si->si_value.sival_ptr;
26-
self->expire_now();
43+
const uintptr_t sival_ptr_as_uint = reinterpret_cast<uintptr_t>(si->si_value.sival_ptr);
44+
const generation_t expiry_gen = sival_ptr_as_uint >> tag_ptr_shift;
45+
platform_timer* self = reinterpret_cast<platform_timer*>(sival_ptr_as_uint & ~tag_ptr_mask);
46+
47+
self->expire_now(expiry_gen);
2748
}
2849
};
2950

3051
platform_timer::platform_timer() {
3152
static_assert(std::atomic<timer_state_t>::is_always_lock_free, "Only lock-free atomics AS-safe.");
3253
static_assert(sizeof(impl) <= fwd_size);
54+
static_assert(std::numeric_limits<generation_t>::max() > impl::num_timers-1, "generation_t rolls over before timer count does");
3355

34-
static bool initialized;
35-
static std::mutex initialized_mutex;
36-
37-
if(std::lock_guard guard(initialized_mutex); !initialized) {
38-
struct sigaction act;
39-
sigemptyset(&act.sa_mask);
40-
act.sa_sigaction = impl::sig_handler;
41-
act.sa_flags = SA_SIGINFO | SA_RESTART;
42-
FC_ASSERT(sigaction(SIGRTMIN, &act, NULL) == 0, "failed to aquire SIGRTMIN signal");
43-
initialized = true;
44-
}
56+
static impl::signal_initer the_signal_initer;
4557

46-
struct sigevent se;
47-
se.sigev_notify = SIGEV_SIGNAL;
48-
se.sigev_signo = SIGRTMIN;
49-
se.sigev_value.sival_ptr = (void*)this;
58+
for(uint64_t i = 0; i < impl::num_timers; ++i) {
59+
struct sigevent se;
60+
se.sigev_notify = SIGEV_SIGNAL;
61+
se.sigev_signo = SIGRTMIN;
62+
se.sigev_value.sival_ptr = (void*)(reinterpret_cast<uintptr_t>(this) | i<<impl::tag_ptr_shift);
5063

51-
FC_ASSERT(timer_create(CLOCK_REALTIME, &se, &my->timerid) == 0, "failed to create timer");
64+
FC_ASSERT(timer_create(CLOCK_REALTIME, &se, &my->timerid[i]) == 0, "failed to create timer");
65+
}
5266

5367
compute_and_print_timer_accuracy(*this);
5468
}
5569

5670
platform_timer::~platform_timer() {
57-
timer_delete(my->timerid);
71+
for(unsigned i = 0; i < impl::num_timers; ++i)
72+
timer_delete(my->timerid[i]);
5873
}
5974

6075
void platform_timer::start(fc::time_point tp) {
6176
assert(timer_state() == state_t::stopped);
77+
generation = (generation + 1) % impl::num_timers;
6278
timer_running_forever = tp == fc::time_point::maximum();
6379
if(timer_running_forever) {
64-
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false});
80+
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false, .generation_running = generation});
6581
return;
6682
}
6783
fc::microseconds x = tp.time_since_epoch() - fc::time_point::now().time_since_epoch();
6884
if(x.count() <= 0) {
69-
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false});
85+
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false, .generation_running = generation});
7086
} else {
7187
time_t secs = x.count() / 1000000;
7288
long nsec = (x.count() - (secs*1000000)) * 1000;
7389
struct itimerspec enable = {{0, 0}, {secs, nsec}};
74-
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false});
75-
if(timer_settime(my->timerid, 0, &enable, NULL) != 0) {
76-
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false});
90+
_state.store(timer_state_t{.state = state_t::running, .callback_in_flight = false, .generation_running = generation});
91+
if(timer_settime(my->timerid[generation], 0, &enable, NULL) != 0) {
92+
_state.store(timer_state_t{.state = state_t::timed_out, .callback_in_flight = false, .generation_running = generation});
7793
}
7894
}
7995
}
8096

81-
void platform_timer::expire_now() {
82-
timer_state_t expected{.state = state_t::running, .callback_in_flight = false};
83-
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::timed_out, true})) {
97+
void platform_timer::expire_now(generation_t expired_generation) {
98+
timer_state_t expected{.state = state_t::running, .callback_in_flight = false, .generation_running = expired_generation};
99+
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::timed_out, true, expired_generation})) {
84100
call_expiration_callback();
85-
_state.store(timer_state_t{state_t::timed_out, false});
101+
_state.store(timer_state_t{state_t::timed_out, false, expired_generation});
86102
}
87103
}
88104

89105
void platform_timer::interrupt_timer() {
90-
timer_state_t expected{.state = state_t::running, .callback_in_flight = false};
91-
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::interrupted, true})) {
106+
const generation_t generation_running = _state.load().generation_running;
107+
timer_state_t expected = {.state = state_t::running, .callback_in_flight = false, .generation_running = generation_running};
108+
if (_state.compare_exchange_strong(expected, timer_state_t{state_t::interrupted, true, generation_running})) {
92109
call_expiration_callback();
93-
_state.store(timer_state_t{state_t::interrupted, false});
110+
_state.store(timer_state_t{state_t::interrupted, false, generation_running});
94111
}
95112
}
96113

97114
void platform_timer::stop() {
98115
// if still running, then interrupt so expire_now() and interrupt_timer() can't start a callback call
99-
timer_state_t prior_state{.state = state_t::running, .callback_in_flight = false};
100-
if (_state.compare_exchange_strong(prior_state, timer_state_t{state_t::interrupted, false})) {
101-
prior_state = timer_state_t{state_t::interrupted, false};
116+
timer_state_t prior_state{.state = state_t::running, .callback_in_flight = false, .generation_running = generation};
117+
if (_state.compare_exchange_strong(prior_state, timer_state_t{state_t::interrupted, false, generation})) {
118+
prior_state = timer_state_t{state_t::interrupted, false, generation};
102119
}
103120

104121
for (; prior_state.callback_in_flight; prior_state = _state.load())
105122
boost::core::sp_thread_pause();
106123

107124
if(prior_state.state == state_t::stopped)
108125
return;
109-
_state.store(timer_state_t{.state = state_t::stopped, .callback_in_flight = false});
126+
_state.store(timer_state_t{.state = state_t::stopped, .callback_in_flight = false, .generation_running = generation});
110127
if(prior_state.state == state_t::timed_out || timer_running_forever)
111128
return;
112129
struct itimerspec disable = {{0, 0}, {0, 0}};
113-
timer_settime(my->timerid, 0, &disable, NULL);
130+
timer_settime(my->timerid[generation], 0, &disable, NULL);
114131
}
115132

116133
}

0 commit comments

Comments
 (0)