mirror of
https://github.com/ChaiScript/ChaiScript.git
synced 2026-04-30 19:09:26 +08:00
* Fix #655: Join async threads before engine destruction to prevent heap-use-after-free Issues #632 and #636 (PRs #651 and #653) both stem from the same root cause: async threads spawned via async() can outlive the Dispatch_Engine, accessing shared state (global objects map, type maps) after it has been destroyed. The fix moves async() registration from the stdlib module into ChaiScript_Basic, where spawned threads are tracked via Dispatch_Engine. The engine's destructor now joins all outstanding async threads before destroying shared data structures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review: follow rule of 5, explicitly default move operations Requested by @lefticus in PR #656 review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: leftibot <leftibot@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
1b27d0dd15
commit
7b95ff5126
@ -413,6 +413,9 @@ 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: async() is registered in ChaiScript_Basic::build_eval_system()
|
||||
// with thread tracking to prevent heap-use-after-free on engine destruction.
|
||||
#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,28 @@ namespace chaiscript {
|
||||
, m_parser(parser) {
|
||||
}
|
||||
|
||||
~Dispatch_Engine() {
|
||||
join_async_threads();
|
||||
}
|
||||
|
||||
Dispatch_Engine(const Dispatch_Engine &) = delete;
|
||||
Dispatch_Engine &operator=(const Dispatch_Engine &) = delete;
|
||||
Dispatch_Engine(Dispatch_Engine &&) = default;
|
||||
Dispatch_Engine &operator=(Dispatch_Engine &&) = default;
|
||||
|
||||
#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
|
||||
|
||||
/// \brief casts an object while applying any Dynamic_Conversion available
|
||||
template<typename Type>
|
||||
decltype(auto) boxed_cast(const Boxed_Value &bv) const {
|
||||
@ -1165,6 +1188,21 @@ namespace chaiscript {
|
||||
get_function_objects_int().insert_or_assign(t_name, std::move(new_func));
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
mutable chaiscript::detail::threading::shared_mutex m_mutex;
|
||||
|
||||
Type_Conversions m_conversions;
|
||||
@ -1174,6 +1212,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 {
|
||||
|
||||
@ -15,6 +15,7 @@
|
||||
#include <exception>
|
||||
#include <fstream>
|
||||
#include <functional>
|
||||
#include <future>
|
||||
#include <map>
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
@ -179,6 +180,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 the engine can join all
|
||||
// async threads before destroying shared state (issues #632, #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
|
||||
|
||||
17
unittests/async_engine_lifetime.chai
Normal file
17
unittests/async_engine_lifetime.chai
Normal file
@ -0,0 +1,17 @@
|
||||
// Regression test for #632 and #636: Heap-use-after-free in async threads
|
||||
// Async threads must complete before the engine is destroyed.
|
||||
|
||||
var func = fun(){
|
||||
var ret = 0;
|
||||
for (var i = 0; i < 1000; ++i) {
|
||||
ret += i;
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
|
||||
var fut1 = async(func);
|
||||
var fut2 = async(func);
|
||||
|
||||
// Wait for results to verify correctness
|
||||
assert_equal(fut1.get(), 499500);
|
||||
assert_equal(fut2.get(), 499500);
|
||||
46
unittests/async_engine_lifetime_test.cpp
Normal file
46
unittests/async_engine_lifetime_test.cpp
Normal file
@ -0,0 +1,46 @@
|
||||
// Regression test for #632 and #636:
|
||||
// Heap-use-after-free when async threads outlive the ChaiScript engine.
|
||||
// The engine must join all outstanding async threads before destroying shared state.
|
||||
|
||||
#include <chaiscript/chaiscript.hpp>
|
||||
|
||||
int main() {
|
||||
// Test 1: Async threads still running when engine is destroyed.
|
||||
// Without the fix, this triggers heap-use-after-free under ASAN/TSAN.
|
||||
for (int iter = 0; iter < 3; ++iter) {
|
||||
{
|
||||
chaiscript::ChaiScript chai;
|
||||
try {
|
||||
chai.eval(R"(
|
||||
var func = fun(){
|
||||
var ret = 0;
|
||||
for (var i = 0; i < 5000; ++i) {
|
||||
ret += i;
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
|
||||
var fut1 = async(func);
|
||||
var fut2 = async(func);
|
||||
)");
|
||||
// Engine destroyed here without waiting for futures.
|
||||
} catch (const std::exception &) {
|
||||
// Exceptions are fine
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Test 2: Verify async still works correctly (results are accessible).
|
||||
{
|
||||
chaiscript::ChaiScript chai;
|
||||
auto result = chai.eval<int>(R"(
|
||||
var f = async(fun() { return 42; });
|
||||
f.get();
|
||||
)");
|
||||
if (result != 42) {
|
||||
return EXIT_FAILURE;
|
||||
}
|
||||
}
|
||||
|
||||
return EXIT_SUCCESS;
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user