Skip to content

Commit 51f3f39

Browse files
committed
Fix use-after-free and add a timeout
1 parent e6e05ed commit 51f3f39

File tree

1 file changed

+28
-14
lines changed

1 file changed

+28
-14
lines changed

module.cc

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ struct ThreadInfo {
4343
// Last time this thread was seen in milliseconds since epoch
4444
milliseconds last_seen;
4545
// Optional async local storage associated with this thread
46-
std::optional<AsyncLocalStorageLookup> async_store;
46+
// Using shared_ptr to safely share with async tasks even if ThreadInfo is
47+
// erased
48+
std::shared_ptr<std::optional<AsyncLocalStorageLookup>> async_store;
4749
// Some JSON serialized state sent via threadPoll
4850
std::string poll_state;
4951
};
@@ -360,6 +362,12 @@ CaptureStackTrace(Isolate *isolate,
360362
isolate->RequestInterrupt(ExecutionInterrupted,
361363
new InterruptArgs{std::move(promise), &store});
362364

365+
// Wait with timeout to prevent infinite hang if isolate never processes
366+
// interrupt (e.g., stuck in native code or terminating)
367+
if (future.wait_for(std::chrono::seconds(5)) == std::future_status::timeout) {
368+
return JsStackTrace{{}, ""};
369+
}
370+
363371
return future.get();
364372
}
365373

@@ -381,17 +389,19 @@ void CaptureStackTraces(const FunctionCallbackInfo<Value> &args) {
381389

382390
auto thread_name = thread_info.thread_name;
383391
auto poll_state = thread_info.poll_state;
384-
385-
futures.emplace_back(std::async(
386-
std::launch::async,
387-
[thread_isolate, thread_name, poll_state](
388-
const std::optional<AsyncLocalStorageLookup> &async_store)
389-
-> ThreadResult {
390-
return ThreadResult{thread_name,
391-
CaptureStackTrace(thread_isolate, async_store),
392-
poll_state};
393-
},
394-
std::cref(thread_info.async_store)));
392+
// Capture shared_ptr to keep async_store alive even if thread is removed
393+
// from map
394+
auto async_store_ptr = thread_info.async_store;
395+
396+
futures.emplace_back(
397+
std::async(std::launch::async,
398+
[thread_isolate, thread_name, poll_state,
399+
async_store_ptr]() -> ThreadResult {
400+
return ThreadResult{
401+
thread_name,
402+
CaptureStackTrace(thread_isolate, *async_store_ptr),
403+
poll_state};
404+
}));
395405
}
396406
}
397407

@@ -516,8 +526,12 @@ void RegisterThreadInternal(
516526
std::lock_guard<std::mutex> lock(threads_mutex);
517527
auto found = threads.find(isolate);
518528
if (found == threads.end()) {
519-
threads.emplace(isolate, ThreadInfo{thread_name, milliseconds::zero(),
520-
std::move(async_store), ""});
529+
threads.emplace(
530+
isolate,
531+
ThreadInfo{thread_name, milliseconds::zero(),
532+
std::make_shared<std::optional<AsyncLocalStorageLookup>>(
533+
std::move(async_store)),
534+
""});
521535
// Register a cleanup hook to remove this thread when the isolate is
522536
// destroyed
523537
node::AddEnvironmentCleanupHook(isolate, Cleanup, isolate);

0 commit comments

Comments
 (0)