Skip to content

Commit e19c500

Browse files
timfishtimfish
committed
fix: Don't send JavaScript objects across isolates
1 parent 5332dbf commit e19c500

File tree

2 files changed

+109
-114
lines changed

2 files changed

+109
-114
lines changed

‎module.cc‎

Lines changed: 109 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -21,120 +21,142 @@ static std::mutex threads_mutex;
2121
// Map to hold all registered threads and their information
2222
static std::unordered_map<v8::Isolate *, ThreadInfo> threads = {};
2323

24+
// Structure to hold stack frame information
25+
struct JsStackFrame {
26+
std::string function_name;
27+
std::string filename;
28+
int lineno;
29+
int colno;
30+
};
31+
2432
// Function to be called when an isolate's execution is interrupted
2533
static void ExecutionInterrupted(Isolate *isolate, void *data) {
26-
auto promise = static_cast<std::promise<Local<Array>> *>(data);
34+
auto promise = static_cast<std::promise<std::vector<JsStackFrame>> *>(data);
2735
auto stack = StackTrace::CurrentStackTrace(isolate, kMaxStackFrames,
2836
StackTrace::kDetailed);
2937

30-
if (stack.IsEmpty()) {
31-
promise->set_value(Array::New(isolate, 0));
32-
return;
33-
}
34-
35-
auto frames = Array::New(isolate, stack->GetFrameCount());
36-
37-
for (int i = 0; i < stack->GetFrameCount(); i++) {
38-
auto frame = stack->GetFrame(isolate, i);
39-
auto fn_name = frame->GetFunctionName();
40-
41-
if (frame->IsEval()) {
42-
fn_name =
43-
String::NewFromUtf8(isolate, "[eval]", NewStringType::kInternalized)
44-
.ToLocalChecked();
45-
} else if (fn_name.IsEmpty() || fn_name->Length() == 0) {
46-
fn_name = String::NewFromUtf8(isolate, "?", NewStringType::kInternalized)
47-
.ToLocalChecked();
48-
} else if (frame->IsConstructor()) {
49-
fn_name = String::NewFromUtf8(isolate, "[constructor]",
50-
NewStringType::kInternalized)
51-
.ToLocalChecked();
38+
std::vector<JsStackFrame> frames;
39+
if (!stack.IsEmpty()) {
40+
for (int i = 0; i < stack->GetFrameCount(); i++) {
41+
auto frame = stack->GetFrame(isolate, i);
42+
auto fn_name = frame->GetFunctionName();
43+
44+
std::string function_name;
45+
if (frame->IsEval()) {
46+
function_name = "[eval]";
47+
} else if (fn_name.IsEmpty() || fn_name->Length() == 0) {
48+
function_name = "?";
49+
} else if (frame->IsConstructor()) {
50+
function_name = "[constructor]";
51+
} else {
52+
v8::String::Utf8Value utf8_fn(isolate, fn_name);
53+
function_name = *utf8_fn ? *utf8_fn : "?";
54+
}
55+
56+
std::string filename;
57+
auto script_name = frame->GetScriptName();
58+
if (!script_name.IsEmpty()) {
59+
v8::String::Utf8Value utf8_filename(isolate, script_name);
60+
filename = *utf8_filename ? *utf8_filename : "<unknown>";
61+
} else {
62+
filename = "<unknown>";
63+
}
64+
65+
int lineno = frame->GetLineNumber();
66+
int colno = frame->GetColumn();
67+
68+
frames.push_back(JsStackFrame{function_name, filename, lineno, colno});
5269
}
53-
54-
auto frame_obj = Object::New(isolate);
55-
frame_obj
56-
->Set(isolate->GetCurrentContext(),
57-
String::NewFromUtf8(isolate, "function",
58-
NewStringType::kInternalized)
59-
.ToLocalChecked(),
60-
fn_name)
61-
.Check();
62-
63-
frame_obj
64-
->Set(isolate->GetCurrentContext(),
65-
String::NewFromUtf8(isolate, "filename",
66-
NewStringType::kInternalized)
67-
.ToLocalChecked(),
68-
frame->GetScriptName())
69-
.Check();
70-
71-
frame_obj
72-
->Set(
73-
isolate->GetCurrentContext(),
74-
String::NewFromUtf8(isolate, "lineno", NewStringType::kInternalized)
75-
.ToLocalChecked(),
76-
Integer::New(isolate, frame->GetLineNumber()))
77-
.Check();
78-
79-
frame_obj
80-
->Set(
81-
isolate->GetCurrentContext(),
82-
String::NewFromUtf8(isolate, "colno", NewStringType::kInternalized)
83-
.ToLocalChecked(),
84-
Integer::New(isolate, frame->GetColumn()))
85-
.Check();
86-
87-
frames->Set(isolate->GetCurrentContext(), i, frame_obj).Check();
8870
}
8971

9072
promise->set_value(frames);
9173
}
9274

9375
// Function to capture the stack trace of a single isolate
94-
Local<Array> CaptureStackTrace(Isolate *isolate) {
95-
std::promise<Local<Array>> promise;
76+
std::future<std::vector<JsStackFrame>> CaptureStackTrace(Isolate *isolate) {
77+
std::promise<std::vector<JsStackFrame>> promise;
9678
auto future = promise.get_future();
97-
98-
// The v8 isolate must be interrupted to capture the stack trace
99-
// Execution resumes automatically after ExecutionInterrupted returns
10079
isolate->RequestInterrupt(ExecutionInterrupted, &promise);
101-
return future.get();
80+
return future;
10281
}
10382

83+
struct ThreadFramesFuture {
84+
std::string thread_name;
85+
std::future<std::vector<JsStackFrame>> frames_future;
86+
};
87+
10488
// Function to capture stack traces from all registered threads
10589
void CaptureStackTraces(const FunctionCallbackInfo<Value> &args) {
10690
auto capture_from_isolate = args.GetIsolate();
10791

108-
using ThreadResult = std::tuple<std::string, Local<Array>>;
109-
std::vector<std::future<ThreadResult>> futures;
92+
std::vector<ThreadFramesFuture> futures;
93+
{
94+
std::lock_guard<std::mutex> lock(threads_mutex);
11095

111-
// We collect the futures into a vec so they can be processed in parallel
112-
std::lock_guard<std::mutex> lock(threads_mutex);
113-
for (auto [thread_isolate, thread_info] : threads) {
114-
if (thread_isolate == capture_from_isolate)
115-
continue;
116-
117-
auto thread_name = thread_info.thread_name;
118-
119-
futures.emplace_back(std::async(
120-
std::launch::async,
121-
[thread_name](Isolate *isolate) -> ThreadResult {
122-
return std::make_tuple(thread_name, CaptureStackTrace(isolate));
123-
},
124-
thread_isolate));
96+
for (auto [thread_isolate, thread_info] : threads) {
97+
if (thread_isolate == capture_from_isolate)
98+
continue;
99+
100+
auto thread_name = thread_info.thread_name;
101+
futures.push_back(
102+
ThreadFramesFuture{thread_name, CaptureStackTrace(thread_isolate)});
103+
}
125104
}
126105

127-
// We wait for all futures to complete and collect their results into a
128-
// JavaScript object
129106
Local<Object> result = Object::New(capture_from_isolate);
130-
for (auto &future : futures) {
131-
auto [thread_name, frames] = future.get();
132-
133-
auto key = String::NewFromUtf8(capture_from_isolate, thread_name.c_str(),
107+
for (auto &thread_future : futures) {
108+
auto frames = thread_future.frames_future.get();
109+
auto key = String::NewFromUtf8(capture_from_isolate,
110+
thread_future.thread_name.c_str(),
134111
NewStringType::kNormal)
135112
.ToLocalChecked();
136113

137-
result->Set(capture_from_isolate->GetCurrentContext(), key, frames).Check();
114+
Local<Array> jsFrames =
115+
Array::New(capture_from_isolate, static_cast<int>(frames.size()));
116+
for (size_t i = 0; i < frames.size(); ++i) {
117+
const auto &f = frames[i];
118+
Local<Object> frameObj = Object::New(capture_from_isolate);
119+
frameObj
120+
->Set(capture_from_isolate->GetCurrentContext(),
121+
String::NewFromUtf8(capture_from_isolate, "function",
122+
NewStringType::kInternalized)
123+
.ToLocalChecked(),
124+
String::NewFromUtf8(capture_from_isolate,
125+
f.function_name.c_str(),
126+
NewStringType::kNormal)
127+
.ToLocalChecked())
128+
.Check();
129+
frameObj
130+
->Set(capture_from_isolate->GetCurrentContext(),
131+
String::NewFromUtf8(capture_from_isolate, "filename",
132+
NewStringType::kInternalized)
133+
.ToLocalChecked(),
134+
String::NewFromUtf8(capture_from_isolate, f.filename.c_str(),
135+
NewStringType::kNormal)
136+
.ToLocalChecked())
137+
.Check();
138+
frameObj
139+
->Set(capture_from_isolate->GetCurrentContext(),
140+
String::NewFromUtf8(capture_from_isolate, "lineno",
141+
NewStringType::kInternalized)
142+
.ToLocalChecked(),
143+
Integer::New(capture_from_isolate, f.lineno))
144+
.Check();
145+
frameObj
146+
->Set(capture_from_isolate->GetCurrentContext(),
147+
String::NewFromUtf8(capture_from_isolate, "colno",
148+
NewStringType::kInternalized)
149+
.ToLocalChecked(),
150+
Integer::New(capture_from_isolate, f.colno))
151+
.Check();
152+
jsFrames
153+
->Set(capture_from_isolate->GetCurrentContext(),
154+
static_cast<uint32_t>(i), frameObj)
155+
.Check();
156+
}
157+
158+
result->Set(capture_from_isolate->GetCurrentContext(), key, jsFrames)
159+
.Check();
138160
}
139161

140162
args.GetReturnValue().Set(result);

‎test/yarn.lock‎

Lines changed: 0 additions & 27 deletions
This file was deleted.

0 commit comments

Comments
 (0)