Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions include/chaiscript/chaiscript_stdlib.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
44 changes: 44 additions & 0 deletions include/chaiscript/dispatchkit/dispatchkit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <stdexcept>
#include <string>
#include <string_view>
#include <thread>
#include <typeinfo>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 24 additions & 0 deletions include/chaiscript/language/chaiscript_engine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
51 changes: 51 additions & 0 deletions unittests/async_engine_lifetime_test.cpp
Original file line number Diff line number Diff line change
@@ -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;
}