Skip to content

Commit

Permalink
Merge pull request #30 from AntelopeIO/threaded_timedout_fix_s10
Browse files Browse the repository at this point in the history 
[spring 1.0] avoid spurious access violation on thread due to other timed out thread
  • Loading branch information
@spoonincode
spoonincode authored Jan 21, 2025
2 parents d111d49 + 1e84cb8 commit 02751e0
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 47 deletions.
13 changes: 9 additions & 4 deletions include/eosio/vm/backend.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,20 +305,25 @@ namespace eosio { namespace vm {

template<typename Watchdog, typename F>
inline void timed_run(Watchdog&& wd, F&& f) {
std::atomic<bool> _timed_out = false;
//timed_run_has_timed_out -- declared in signal handling code because signal handler needs to inspect it on a SEGV too -- is a thread local
// so that upon a SEGV the signal handling code can discern if the thread that caused the SEGV has a timed_run that has timed out. This
// thread local also need to be an atomic because the thread that a Watchdog callback will be called from may not be the same as the
// executing thread.
std::atomic<bool>& _timed_out = timed_run_has_timed_out;
auto reenable_code = scope_guard{[&](){
if (_timed_out) {
if (_timed_out.load(std::memory_order_acquire)) {
mod->allocator.enable_code(Impl::is_jit);
_timed_out.store(false, std::memory_order_release);
}
}};
try {
auto wd_guard = wd.scoped_run([this,&_timed_out]() {
_timed_out = true;
_timed_out.store(true, std::memory_order_release);
mod->allocator.disable_code();
});
static_cast<F&&>(f)();
} catch(wasm_memory_exception&) {
if (_timed_out) {
if (_timed_out.load(std::memory_order_acquire)) {
throw timeout_exception{ "execution timed out" };
} else {
throw;
Expand Down
6 changes: 3 additions & 3 deletions include/eosio/vm/execution_context.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,11 @@ namespace eosio { namespace vm {

vm::invoke_with_signal_handler([&]() {
result = execute<sizeof...(Args)>(args_raw, fn, this, base_type::linear_memory(), stack);
}, &handle_signal, {_mod->allocator.get_code_span(), base_type::get_wasm_allocator()->get_span()});
}, &handle_signal, _mod->allocator, base_type::get_wasm_allocator());
} else {
vm::invoke_with_signal_handler([&]() {
result = execute<sizeof...(Args)>(args_raw, fn, this, base_type::linear_memory(), stack);
}, &handle_signal, {_mod->allocator.get_code_span(), base_type::get_wasm_allocator()->get_span()});
}, &handle_signal, _mod->allocator, base_type::get_wasm_allocator());
}
}
} catch(wasm_exit_exception&) {
Expand Down Expand Up @@ -800,7 +800,7 @@ namespace eosio { namespace vm {
setup_locals(func_index);
vm::invoke_with_signal_handler([&]() {
execute(visitor);
}, &handle_signal, {_mod->allocator.get_code_span(), base_type::get_wasm_allocator()->get_span()});
}, &handle_signal, _mod->allocator, base_type::get_wasm_allocator());
}

if (_mod->get_function_type(func_index).return_count && !_state.exiting) {
Expand Down
92 changes: 54 additions & 38 deletions include/eosio/vm/signals.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ namespace eosio { namespace vm {
inline thread_local std::atomic<sigjmp_buf*> signal_dest{nullptr};

__attribute__((visibility("default")))
inline thread_local std::vector<std::span<std::byte>> protected_memory_ranges;
inline thread_local std::span<std::byte> code_memory_range;

__attribute__((visibility("default")))
inline thread_local std::span<std::byte> memory_range;

__attribute__((visibility("default")))
inline thread_local std::atomic<bool> timed_run_has_timed_out{false};

// Fixes a duplicate symbol build issue when building with `-fvisibility=hidden`
__attribute__((visibility("default")))
Expand All @@ -27,46 +33,55 @@ namespace eosio { namespace vm {
template<int Sig>
inline struct sigaction prev_signal_handler;

inline bool in_protected_range(void* addr) {
//empty protection list means legacy catch-all behavior; useful for some of the old tests
if(protected_memory_ranges.empty())
return true;

for(const std::span<std::byte>& range : protected_memory_ranges) {
if(addr >= range.data() && addr < range.data() + range.size())
return true;
}
return false;
}

inline void signal_handler(int sig, siginfo_t* info, void* uap) {
sigjmp_buf* dest = std::atomic_load(&signal_dest);

if (dest && in_protected_range(info->si_addr)) {
siglongjmp(*dest, sig);
} else {
struct sigaction* prev_action;
switch(sig) {
case SIGSEGV: prev_action = &prev_signal_handler<SIGSEGV>; break;
case SIGBUS: prev_action = &prev_signal_handler<SIGBUS>; break;
case SIGFPE: prev_action = &prev_signal_handler<SIGFPE>; break;
default: std::abort();
if (dest) {
const void* addr = info->si_addr;

//neither range set means legacy catch-all behavior; useful for some of the old tests
if (code_memory_range.empty() && memory_range.empty())
siglongjmp(*dest, sig);

//a failure in the memory range is always jumped out of
if (addr >= memory_range.data() && addr < memory_range.data() + memory_range.size())
siglongjmp(*dest, sig);

//a failure in the code range...
if (addr >= code_memory_range.data() && addr < code_memory_range.data() + code_memory_range.size()) {
//a SEGV in the code range when timed_run_has_timed_out=false is due to a _different_ thread's execution activating a deadline
// timer. Return and retry executing the same code again. Eventually timed_run() on the other thread will reset the page
// permissions and progress on this thread can continue
if (sig == SIGSEGV && timed_run_has_timed_out.load(std::memory_order_acquire) == false)
return;
//otherwise, jump out
siglongjmp(*dest, sig);
}
if (!prev_action) std::abort();
if (prev_action->sa_flags & SA_SIGINFO) {
// FIXME: We need to be at least as strict as the original
// flags and relax the mask as needed.
prev_action->sa_sigaction(sig, info, uap);

//if in neither range, fall through and let chained handler an opportunity to handle
}

struct sigaction* prev_action;
switch(sig) {
case SIGSEGV: prev_action = &prev_signal_handler<SIGSEGV>; break;
case SIGBUS: prev_action = &prev_signal_handler<SIGBUS>; break;
case SIGFPE: prev_action = &prev_signal_handler<SIGFPE>; break;
default: std::abort();
}
if (!prev_action) std::abort();
if (prev_action->sa_flags & SA_SIGINFO) {
// FIXME: We need to be at least as strict as the original
// flags and relax the mask as needed.
prev_action->sa_sigaction(sig, info, uap);
} else {
if(prev_action->sa_handler == SIG_DFL) {
// The default for all three signals is to terminate the process.
sigaction(sig, prev_action, nullptr);
raise(sig);
} else if(prev_action->sa_handler == SIG_IGN) {
// Do nothing
} else {
if(prev_action->sa_handler == SIG_DFL) {
// The default for all three signals is to terminate the process.
sigaction(sig, prev_action, nullptr);
raise(sig);
} else if(prev_action->sa_handler == SIG_IGN) {
// Do nothing
} else {
prev_action->sa_handler(sig);
}
prev_action->sa_handler(sig);
}
}
}
Expand Down Expand Up @@ -139,11 +154,12 @@ namespace eosio { namespace vm {
// It's unlikely, but I'm not sure that it can definitely be ruled out if both
// this and f are inlined and f modifies locals from the caller.
template<typename F, typename E>
[[gnu::noinline]] auto invoke_with_signal_handler(F&& f, E&& e, const std::vector<std::span<std::byte>>& protect_ranges) {
[[gnu::noinline]] auto invoke_with_signal_handler(F&& f, E&& e, growable_allocator& code_allocator, wasm_allocator* mem_allocator) {
setup_signal_handler();
sigjmp_buf dest;
sigjmp_buf* volatile old_signal_handler = nullptr;
protected_memory_ranges = protect_ranges;
code_memory_range = code_allocator.get_code_span();
memory_range = mem_allocator->get_span();
int sig;
if((sig = sigsetjmp(dest, 1)) == 0) {
// Note: Cannot use RAII, as non-trivial destructors w/ longjmp
Expand Down
4 changes: 2 additions & 2 deletions tests/signals_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ TEST_CASE("Testing signals", "[invoke_with_signal_handler]") {
std::raise(SIGSEGV);
}, [](int sig) {
throw test_exception{};
}, {});
}, {}, {});
} catch(test_exception&) {
okay = true;
}
Expand All @@ -25,7 +25,7 @@ TEST_CASE("Testing signals", "[invoke_with_signal_handler]") {
TEST_CASE("Testing throw", "[signal_handler_throw]") {
CHECK_THROWS_AS(eosio::vm::invoke_with_signal_handler([](){
eosio::vm::throw_<eosio::vm::wasm_exit_exception>( "Exiting" );
}, [](int){}, {}), eosio::vm::wasm_exit_exception);
}, [](int){}, {}, {}), eosio::vm::wasm_exit_exception);
}

static volatile sig_atomic_t sig_handled;
Expand Down

0 comments on commit 02751e0

Please sign in to comment.