From c5b81b880fd02334d7ee9293fa6d0d6dce2b2cb3 Mon Sep 17 00:00:00 2001 From: leftibot Date: Sat, 11 Apr 2026 09:24:10 -0600 Subject: [PATCH] Fix #632: Join async threads before engine destruction to prevent use-after-free The heap-use-after-free was caused by a race condition where async threads continued accessing the engine's shared state (global objects map) after the main thread began destroying the Dispatch_Engine. The fix moves the async function registration from the stdlib module into ChaiScript_Basic, where it can track spawned threads 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) --- CMakeLists.txt | 4 ++ include/chaiscript/chaiscript_stdlib.hpp | 3 -- .../chaiscript/dispatchkit/dispatchkit.hpp | 35 ++++++++++++++++ .../chaiscript/language/chaiscript_engine.hpp | 18 ++++++++ unittests/async_engine_lifetime.chai | 18 ++++++++ unittests/async_lifetime_test.cpp | 42 +++++++++++++++++++ 6 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 unittests/async_engine_lifetime.chai create mode 100644 unittests/async_lifetime_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 878749ad..64763ca5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -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_lifetime_test unittests/async_lifetime_test.cpp) + target_link_libraries(async_lifetime_test ${LIBS}) + add_test(NAME Async_Lifetime_Test COMMAND async_lifetime_test) endif() add_executable(multifile_test diff --git a/include/chaiscript/chaiscript_stdlib.hpp b/include/chaiscript/chaiscript_stdlib.hpp index 5cb4fa68..24ec9f1b 100644 --- a/include/chaiscript/chaiscript_stdlib.hpp +++ b/include/chaiscript/chaiscript_stdlib.hpp @@ -49,9 +49,6 @@ namespace chaiscript { #ifndef CHAISCRIPT_NO_THREADS bootstrap::standard_library::future_type>("future", *lib); - lib->add(chaiscript::fun( - [](const std::function &t_func) { return std::async(std::launch::async, t_func); }), - "async"); #endif json_wrap::library(*lib); diff --git a/include/chaiscript/dispatchkit/dispatchkit.hpp b/include/chaiscript/dispatchkit/dispatchkit.hpp index b54d42b4..d80ef585 100644 --- a/include/chaiscript/dispatchkit/dispatchkit.hpp +++ b/include/chaiscript/dispatchkit/dispatchkit.hpp @@ -370,6 +370,21 @@ namespace chaiscript { , m_parser(parser) { } + ~Dispatch_Engine() { + join_async_threads(); + } + + Dispatch_Engine(const Dispatch_Engine &) = delete; + Dispatch_Engine &operator=(const Dispatch_Engine &) = delete; + +#ifndef CHAISCRIPT_NO_THREADS + /// Track a thread created by async so we can join it before destruction + void track_async_thread(std::thread &&t_thread) { + chaiscript::detail::threading::unique_lock l(m_async_mutex); + m_async_threads.push_back(std::move(t_thread)); + } +#endif + /// \brief casts an object while applying any Dynamic_Conversion available template decltype(auto) boxed_cast(const Boxed_Value &bv) const { @@ -1165,8 +1180,28 @@ 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 threads; + { + chaiscript::detail::threading::unique_lock 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; +#ifndef CHAISCRIPT_NO_THREADS + mutable chaiscript::detail::threading::shared_mutex m_async_mutex; + std::vector m_async_threads; +#endif + Type_Conversions m_conversions; chaiscript::detail::threading::Thread_Storage m_stack_holder; std::reference_wrapper m_parser; diff --git a/include/chaiscript/language/chaiscript_engine.hpp b/include/chaiscript/language/chaiscript_engine.hpp index c9f22e58..618b6742 100644 --- a/include/chaiscript/language/chaiscript_engine.hpp +++ b/include/chaiscript/language/chaiscript_engine.hpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -179,6 +180,23 @@ namespace chaiscript { }), "namespace"); m_engine.add(fun([this](const std::string &t_namespace_name) { import(t_namespace_name); }), "import"); + +#ifndef CHAISCRIPT_NO_THREADS + m_engine.add(chaiscript::fun( + [this](const std::function &t_func) { + auto promise_ptr = std::make_shared>(); + 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 diff --git a/unittests/async_engine_lifetime.chai b/unittests/async_engine_lifetime.chai new file mode 100644 index 00000000..6a81dd03 --- /dev/null +++ b/unittests/async_engine_lifetime.chai @@ -0,0 +1,18 @@ +// Regression test for #632: Heap-use-after-free in async threads +// Async threads must complete before the engine is destroyed. +// Without the fix, this crashes with ASAN (use-after-free). + +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); diff --git a/unittests/async_lifetime_test.cpp b/unittests/async_lifetime_test.cpp new file mode 100644 index 00000000..5db9bf47 --- /dev/null +++ b/unittests/async_lifetime_test.cpp @@ -0,0 +1,42 @@ +// Regression test for #632: Heap-use-after-free in async threads during engine shutdown. +// The engine must wait for outstanding async threads before destroying shared state. + +#include +#include +#include + +int main() { + // Test 1: Async threads that are still running when the engine is destroyed. + // Without the fix, this triggers a heap-use-after-free under ASAN/TSAN. + { + chaiscript::ChaiScript chai; + 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 is destroyed here without explicitly waiting for futures. + // The fix ensures the engine joins all async threads before destruction. + } + + // Test 2: Verify async still works correctly (results are accessible). + { + chaiscript::ChaiScript chai; + auto result = chai.eval(R"( + var f = async(fun() { return 42; }); + f.get(); + )"); + if (result != 42) { + return EXIT_FAILURE; + } + } + + return EXIT_SUCCESS; +}