Skip to content

Commit 0bbd134

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

File tree

1 file changed

+104
-79
lines changed

1 file changed

+104
-79
lines changed

‎module.cc‎

Lines changed: 104 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -21,78 +21,60 @@ 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::vector<JsStackFrame> CaptureStackTrace(Isolate *isolate) {
77+
std::promise<std::vector<JsStackFrame>> promise;
9678
auto future = promise.get_future();
9779

9880
// The v8 isolate must be interrupted to capture the stack trace
@@ -105,36 +87,79 @@ Local<Array> CaptureStackTrace(Isolate *isolate) {
10587
void CaptureStackTraces(const FunctionCallbackInfo<Value> &args) {
10688
auto capture_from_isolate = args.GetIsolate();
10789

108-
using ThreadResult = std::tuple<std::string, Local<Array>>;
90+
using ThreadResult = std::tuple<std::string, std::vector<JsStackFrame>>;
10991
std::vector<std::future<ThreadResult>> futures;
11092

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));
93+
{
94+
std::lock_guard<std::mutex> lock(threads_mutex);
95+
for (auto [thread_isolate, thread_info] : threads) {
96+
if (thread_isolate == capture_from_isolate)
97+
continue;
98+
auto thread_name = thread_info.thread_name;
99+
100+
futures.emplace_back(std::async(
101+
std::launch::async,
102+
[thread_name](Isolate *isolate) -> ThreadResult {
103+
return std::make_tuple(thread_name, CaptureStackTrace(isolate));
104+
},
105+
thread_isolate));
106+
}
125107
}
126108

127-
// We wait for all futures to complete and collect their results into a
128-
// JavaScript object
129109
Local<Object> result = Object::New(capture_from_isolate);
110+
130111
for (auto &future : futures) {
131112
auto [thread_name, frames] = future.get();
132-
133113
auto key = String::NewFromUtf8(capture_from_isolate, thread_name.c_str(),
134114
NewStringType::kNormal)
135115
.ToLocalChecked();
136116

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

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

0 commit comments

Comments
 (0)