diff --git a/CMakeLists.txt b/CMakeLists.txt index 878749ad..e817210d 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_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 diff --git a/include/chaiscript/chaiscript_stdlib.hpp b/include/chaiscript/chaiscript_stdlib.hpp index 5cb4fa68..d1b95ff9 100644 --- a/include/chaiscript/chaiscript_stdlib.hpp +++ b/include/chaiscript/chaiscript_stdlib.hpp @@ -49,9 +49,8 @@ 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"); + // 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); diff --git a/include/chaiscript/dispatchkit/dispatchkit.hpp b/include/chaiscript/dispatchkit/dispatchkit.hpp index b54d42b4..73dc3e42 100644 --- a/include/chaiscript/dispatchkit/dispatchkit.hpp +++ b/include/chaiscript/dispatchkit/dispatchkit.hpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -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 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 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 + } + /// \brief casts an object while applying any Dynamic_Conversion available template 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 m_async_threads; +#endif }; class Dispatch_State { diff --git a/include/chaiscript/language/chaiscript_engine.hpp b/include/chaiscript/language/chaiscript_engine.hpp index c9f22e58..d5dbe3c4 100644 --- a/include/chaiscript/language/chaiscript_engine.hpp +++ b/include/chaiscript/language/chaiscript_engine.hpp @@ -22,6 +22,11 @@ #include #include +#ifndef CHAISCRIPT_NO_THREADS +#include +#include +#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 &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_test.cpp b/unittests/async_engine_lifetime_test.cpp new file mode 100644 index 00000000..2a609238 --- /dev/null +++ b/unittests/async_engine_lifetime_test.cpp @@ -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 +#include + +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; +}