Skip to content

Commit

Permalink
fix: JS values being leaked
Browse files Browse the repository at this point in the history
* JSContextRef frees context and runtime when it's dropped
* JSValueRef is no longer Copy, increases reference count on clone and decreases it on drop
* reworke closures using double-boxing so that they use way less global state
* add tests for bugs stemming from not-freeing or double-freeing
  • Loading branch information
TheEdward162 committed Dec 7, 2023
1 parent a5c49a9 commit 65d69e3
Show file tree
Hide file tree
Showing 16 changed files with 552 additions and 390 deletions.
4 changes: 2 additions & 2 deletions crates/apis/src/console/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ where

fn console_log_to<T>(
mut stream: T,
) -> impl FnMut(&JSContextRef, JSValueRef, &[JSValueRef]) -> Result<JSValue>
) -> impl FnMut(&JSContextRef, &JSValueRef, &[JSValueRef]) -> Result<JSValue>
where
T: Write + 'static,
{
move |_ctx: &JSContextRef, _this: JSValueRef, args: &[JSValueRef]| {
move |_ctx: &JSContextRef, _this: &JSValueRef, args: &[JSValueRef]| {
// Write full string to in-memory destination before writing to stream since each write call to the stream
// will invoke a hostcall.
let mut log_line = String::new();
Expand Down
20 changes: 10 additions & 10 deletions crates/apis/src/text_encoding/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,17 @@ impl JSApiSet for TextEncoding {
}

fn decode_utf8_buffer_to_js_string(
) -> impl FnMut(&JSContextRef, JSValueRef, &[JSValueRef]) -> anyhow::Result<JSValue> {
move |_ctx: &JSContextRef, _this: JSValueRef, args: &[JSValueRef]| {
) -> impl FnMut(&JSContextRef, &JSValueRef, &[JSValueRef]) -> anyhow::Result<JSValue> {
move |_ctx: &JSContextRef, _this: &JSValueRef, args: &[JSValueRef]| {
if args.len() != 5 {
return Err(anyhow!("Expecting 5 arguments, received {}", args.len()));
}

let buffer: Vec<u8> = args[0].try_into()?;
let byte_offset: usize = args[1].try_into()?;
let byte_length: usize = args[2].try_into()?;
let fatal: bool = args[3].try_into()?;
let ignore_bom: bool = args[4].try_into()?;
let buffer: Vec<u8> = (&args[0]).try_into()?;
let byte_offset: usize = (&args[1]).try_into()?;
let byte_length: usize = (&args[2]).try_into()?;
let fatal: bool = (&args[3]).try_into()?;
let ignore_bom: bool = (&args[4]).try_into()?;

let mut view = buffer
.get(byte_offset..(byte_offset + byte_length))
Expand Down Expand Up @@ -68,13 +68,13 @@ fn decode_utf8_buffer_to_js_string(
}

fn encode_js_string_to_utf8_buffer(
) -> impl FnMut(&JSContextRef, JSValueRef, &[JSValueRef]) -> anyhow::Result<JSValue> {
move |_ctx: &JSContextRef, _this: JSValueRef, args: &[JSValueRef]| {
) -> impl FnMut(&JSContextRef, &JSValueRef, &[JSValueRef]) -> anyhow::Result<JSValue> {
move |_ctx: &JSContextRef, _this: &JSValueRef, args: &[JSValueRef]| {
if args.len() != 1 {
return Err(anyhow!("Expecting 1 argument, got {}", args.len()));
}

let js_string: String = args[0].try_into()?;
let js_string: String = (&args[0]).try_into()?;
Ok(js_string.into_bytes().into())
}
}
Expand Down
Loading

0 comments on commit 65d69e3

Please sign in to comment.