mirror of
https://github.com/ChaiScript/ChaiScript.git
synced 2026-07-30 16:26:28 +08:00
Fix #636: Prevent heap-use-after-free in async threads during engine destruction
The Dispatch_Engine member destruction order destroyed m_state (containing type maps) before m_stack_holder (containing async futures), allowing async threads to access freed type map memory during eval_error formatting. The fix moves async() registration from the stdlib into ChaiScript_Basic with thread tracking, and adds a Dispatch_Engine destructor that joins all tracked async threads before m_state is destroyed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7b45fe19fe
commit
96cae78242
@ -403,6 +403,10 @@ if(BUILD_TESTING)
|
||||
"CHAI_USE_PATH=${CMAKE_CURRENT_SOURCE_DIR}/unittests/"
|
||||
"CHAI_MODULE_PATH=${CMAKE_CURRENT_BINARY_DIR}/"
|
||||
)
|
||||
|
||||
add_executable(async_engine_lifetime_test unittests/async_engine_lifetime_test.cpp)
|
||||
target_link_libraries(async_engine_lifetime_test ${LIBS})
|
||||
add_test(NAME Async_Engine_Lifetime_Test COMMAND async_engine_lifetime_test)
|
||||
endif()
|
||||
|
||||
add_executable(multifile_test
|
||||
|
||||
@ -49,9 +49,8 @@ namespace chaiscript {
|
||||
|
||||
#ifndef CHAISCRIPT_NO_THREADS
|
||||
bootstrap::standard_library::future_type<std::future<chaiscript::Boxed_Value>>("future", *lib);
|
||||
lib->add(chaiscript::fun(
|
||||
[](const std::function<chaiscript::Boxed_Value()> &t_func) { return std::async(std::launch::async, t_func); }),
|
||||
"async");
|
||||
// Note: the async() function is registered in ChaiScript_Basic::build_eval_system()
|
||||
// with thread tracking to prevent heap-use-after-free on engine destruction (issue #636).
|
||||
#endif
|
||||
|
||||
json_wrap::library(*lib);
|
||||
|
||||
@ -19,6 +19,7 @@
|
||||
#include <stdexcept>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
#include <thread>
|
||||
#include <typeinfo>
|
||||
#include <utility>
|
||||
#include <vector>
|
||||
@ -370,6 +371,44 @@ namespace chaiscript {
|
||||
, m_parser(parser) {
|
||||
}
|
||||
|
||||
~Dispatch_Engine() {
|
||||
// Join all tracked async threads before m_state is destroyed,
|
||||
// to prevent heap-use-after-free when async threads access type maps
|
||||
// during error formatting (issue #636).
|
||||
join_async_threads();
|
||||
}
|
||||
|
||||
Dispatch_Engine(const Dispatch_Engine &) = delete;
|
||||
Dispatch_Engine &operator=(const Dispatch_Engine &) = delete;
|
||||
|
||||
#ifndef CHAISCRIPT_NO_THREADS
|
||||
/// Track an async thread so it can be joined during destruction
|
||||
void track_async_thread(std::thread t_thread) {
|
||||
chaiscript::detail::threading::unique_lock<chaiscript::detail::threading::shared_mutex> l(m_async_mutex);
|
||||
// Clean up already-finished threads to avoid unbounded growth
|
||||
m_async_threads.erase(
|
||||
std::remove_if(m_async_threads.begin(), m_async_threads.end(),
|
||||
[](std::thread &t) { return !t.joinable(); }),
|
||||
m_async_threads.end());
|
||||
m_async_threads.push_back(std::move(t_thread));
|
||||
}
|
||||
#endif
|
||||
|
||||
void join_async_threads() {
|
||||
#ifndef CHAISCRIPT_NO_THREADS
|
||||
std::vector<std::thread> threads;
|
||||
{
|
||||
chaiscript::detail::threading::unique_lock<chaiscript::detail::threading::shared_mutex> l(m_async_mutex);
|
||||
threads = std::move(m_async_threads);
|
||||
}
|
||||
for (auto &t : threads) {
|
||||
if (t.joinable()) {
|
||||
t.join();
|
||||
}
|
||||
}
|
||||
#endif
|
||||
}
|
||||
|
||||
/// \brief casts an object while applying any Dynamic_Conversion available
|
||||
template<typename Type>
|
||||
decltype(auto) boxed_cast(const Boxed_Value &bv) const {
|
||||
@ -1174,6 +1213,11 @@ namespace chaiscript {
|
||||
mutable std::atomic_uint_fast32_t m_method_missing_loc = {0};
|
||||
|
||||
State m_state;
|
||||
|
||||
#ifndef CHAISCRIPT_NO_THREADS
|
||||
mutable chaiscript::detail::threading::shared_mutex m_async_mutex;
|
||||
std::vector<std::thread> m_async_threads;
|
||||
#endif
|
||||
};
|
||||
|
||||
class Dispatch_State {
|
||||
|
||||
@ -22,6 +22,11 @@
|
||||
#include <stdexcept>
|
||||
#include <vector>
|
||||
|
||||
#ifndef CHAISCRIPT_NO_THREADS
|
||||
#include <future>
|
||||
#include <thread>
|
||||
#endif
|
||||
|
||||
#include "../chaiscript_defines.hpp"
|
||||
#include "../chaiscript_threading.hpp"
|
||||
#include "../dispatchkit/boxed_cast_helper.hpp"
|
||||
@ -179,6 +184,25 @@ namespace chaiscript {
|
||||
}),
|
||||
"namespace");
|
||||
m_engine.add(fun([this](const std::string &t_namespace_name) { import(t_namespace_name); }), "import");
|
||||
|
||||
#ifndef CHAISCRIPT_NO_THREADS
|
||||
// Register async() with thread tracking so that Dispatch_Engine's destructor
|
||||
// can join all async threads before destroying shared state (issue #636).
|
||||
m_engine.add(chaiscript::fun(
|
||||
[this](const std::function<chaiscript::Boxed_Value()> &t_func) {
|
||||
auto promise_ptr = std::make_shared<std::promise<chaiscript::Boxed_Value>>();
|
||||
auto future = promise_ptr->get_future();
|
||||
m_engine.track_async_thread(std::thread([promise_ptr, t_func]() {
|
||||
try {
|
||||
promise_ptr->set_value(t_func());
|
||||
} catch (...) {
|
||||
promise_ptr->set_exception(std::current_exception());
|
||||
}
|
||||
}));
|
||||
return future;
|
||||
}),
|
||||
"async");
|
||||
#endif
|
||||
}
|
||||
|
||||
/// Skip BOM at the beginning of file
|
||||
|
||||
51
unittests/async_engine_lifetime_test.cpp
Normal file
51
unittests/async_engine_lifetime_test.cpp
Normal file
@ -0,0 +1,51 @@
|
||||
// Regression test for issue #636:
|
||||
// Heap-use-after-free when async threads access engine state during destruction.
|
||||
//
|
||||
// The root cause: Dispatch_Engine member destruction order destroys m_state (type maps)
|
||||
// before m_stack_holder (which holds async futures). When async threads are still running
|
||||
// and hit an error that triggers eval_error formatting, they access the already-freed
|
||||
// type map via get_type_name().
|
||||
|
||||
#include <chaiscript/chaiscript.hpp>
|
||||
#include <iostream>
|
||||
|
||||
int main() {
|
||||
// Run multiple iterations to increase chance of triggering the race
|
||||
for (int iter = 0; iter < 3; ++iter) {
|
||||
// Create engine in a nested scope so it's destroyed while async tasks may still be running
|
||||
{
|
||||
chaiscript::ChaiScript chai;
|
||||
|
||||
try {
|
||||
// This script launches async tasks that run a long-running loop.
|
||||
// The futures are not retrieved - when the engine is destroyed, the
|
||||
// async threads may still be accessing engine state (e.g. type maps
|
||||
// during error formatting). Without the fix, m_state is destroyed
|
||||
// before m_stack_holder (which holds the futures), causing the async
|
||||
// threads to access freed memory.
|
||||
//
|
||||
// With ASan/TSan enabled, this reliably detects the heap-use-after-free.
|
||||
chai.eval(R"(
|
||||
var func = fun(){
|
||||
var ret = 0;
|
||||
for (var i = 0; i < 100000; ++i) {
|
||||
ret += i;
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
|
||||
var fut1 = async(func);
|
||||
var fut2 = async(func);
|
||||
)");
|
||||
} catch (const std::exception &) {
|
||||
// Exceptions from eval are expected and fine
|
||||
}
|
||||
}
|
||||
// If the fix is not applied, the engine destruction above may cause
|
||||
// heap-use-after-free (detectable with ASan/TSan) because m_state
|
||||
// is destroyed before async threads finish.
|
||||
}
|
||||
|
||||
std::cout << "Async engine lifetime test passed\n";
|
||||
return EXIT_SUCCESS;
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user