Skip to content

Commit 54b31e6

Browse files
committed
Fix memory leak in Function::GetScriptOrigin
1 parent 5f7713c commit 54b31e6

File tree

4 files changed

+30
-21
lines changed

4 files changed

+30
-21
lines changed

src/binding.cc

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2296,11 +2296,9 @@ int v8__Function__ScriptId(const v8::Function& self) {
22962296
return ptr_to_local(&self)->ScriptId();
22972297
}
22982298

2299-
const v8::ScriptOrigin* v8__Function__GetScriptOrigin(
2300-
const v8::Function& self) {
2301-
std::unique_ptr<v8::ScriptOrigin> u = std::make_unique<v8::ScriptOrigin>(
2302-
ptr_to_local(&self)->GetScriptOrigin());
2303-
return u.release();
2299+
void v8__Function__GetScriptOrigin(
2300+
const v8::Function& self, uninit_t<v8::ScriptOrigin>* out) {
2301+
construct_in_place<v8::ScriptOrigin>(out, ptr_to_local(&self)->GetScriptOrigin());
23042302
}
23052303

23062304
const v8::Signature* v8__Signature__New(v8::Isolate* isolate,
@@ -2713,16 +2711,16 @@ void v8__ScriptOrigin__CONSTRUCT(
27132711
ptr_to_local(host_defined_options));
27142712
}
27152713

2716-
int v8__ScriptOrigin__ScriptId(const v8::ScriptOrigin& self) {
2717-
return ptr_to_local(&self)->ScriptId();
2714+
int v8__ScriptOrigin__ScriptId(const v8::ScriptOrigin* self) {
2715+
return self->ScriptId();
27182716
}
27192717

2720-
const v8::Value* v8__ScriptOrigin__ResourceName(const v8::ScriptOrigin& self) {
2721-
return local_to_ptr(ptr_to_local(&self)->ResourceName());
2718+
const v8::Value* v8__ScriptOrigin__ResourceName(const v8::ScriptOrigin* self) {
2719+
return local_to_ptr(self->ResourceName());
27222720
}
27232721

2724-
const v8::Value* v8__ScriptOrigin__SourceMapUrl(const v8::ScriptOrigin& self) {
2725-
return local_to_ptr(ptr_to_local(&self)->SourceMapUrl());
2722+
const v8::Value* v8__ScriptOrigin__SourceMapUrl(const v8::ScriptOrigin* self) {
2723+
return local_to_ptr(self->SourceMapUrl());
27262724
}
27272725

27282726
const v8::Value* v8__ScriptOrModule__GetResourceName(

src/function.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::convert::TryFrom;
22
use std::marker::PhantomData;
3+
use std::mem::MaybeUninit;
34
use std::ptr::NonNull;
45
use std::ptr::null;
56

@@ -58,9 +59,10 @@ unsafe extern "C" {
5859
fn v8__Function__GetScriptColumnNumber(this: *const Function) -> int;
5960
fn v8__Function__GetScriptLineNumber(this: *const Function) -> int;
6061
fn v8__Function__ScriptId(this: *const Function) -> int;
61-
fn v8__Function__GetScriptOrigin(
62+
fn v8__Function__GetScriptOrigin<'a>(
6263
this: *const Function,
63-
) -> *const ScriptOrigin<'static>;
64+
out: *mut MaybeUninit<ScriptOrigin<'a>>,
65+
);
6466

6567
fn v8__Function__CreateCodeCache(
6668
script: *const Function,
@@ -1085,10 +1087,15 @@ impl Function {
10851087
}
10861088

10871089
#[inline(always)]
1088-
pub fn get_script_origin(&self) -> &ScriptOrigin<'_> {
1090+
pub fn get_script_origin<'s>(
1091+
&self,
1092+
_scope: &PinScope<'s, '_>,
1093+
) -> ScriptOrigin<'s> {
10891094
unsafe {
1090-
let ptr = v8__Function__GetScriptOrigin(self);
1091-
&*ptr
1095+
let mut script_origin: MaybeUninit<ScriptOrigin<'_>> =
1096+
MaybeUninit::uninit();
1097+
v8__Function__GetScriptOrigin(self, &mut script_origin);
1098+
script_origin.assume_init()
10921099
}
10931100
}
10941101

src/script.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use std::marker::PhantomData;
21
use std::mem::MaybeUninit;
32
use std::ptr::null;
43

@@ -16,7 +15,8 @@ use crate::scope::PinScope;
1615
#[derive(Debug)]
1716
pub struct ScriptOrigin<'s>(
1817
[u8; crate::binding::v8__ScriptOrigin_SIZE],
19-
PhantomData<&'s ()>,
18+
// require pointer alignment
19+
[&'s (); 0],
2020
);
2121

2222
unsafe extern "C" {

tests/test_api.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4153,22 +4153,26 @@ fn function_script_origin_and_id() {
41534153

41544154
if let Some(id) = prev_id {
41554155
assert_eq!(script_id, id + 1);
4156-
assert_eq!(script_id, f_function_obj.get_script_origin().script_id(),);
4156+
assert_eq!(
4157+
script_id,
4158+
f_function_obj.get_script_origin(scope).script_id(),
4159+
);
41574160
}
41584161
prev_id = Some(script_id);
41594162

41604163
// Verify source map URL matches
41614164
assert_eq!(
41624165
"source_map_url",
41634166
f_function_obj
4164-
.get_script_origin()
4167+
.get_script_origin(scope)
41654168
.source_map_url()
41664169
.unwrap()
41674170
.to_rust_string_lossy(scope)
41684171
);
41694172

41704173
// Verify resource name matches in script origin
4171-
let resource_name_val = f_function_obj.get_script_origin().resource_name();
4174+
let resource_name_val =
4175+
f_function_obj.get_script_origin(scope).resource_name();
41724176
assert!(resource_name_val.is_some());
41734177
assert_eq!(
41744178
resource_name_val.unwrap().to_rust_string_lossy(scope),

0 commit comments

Comments
 (0)