Skip to content

Commit

Permalink
fix: Rework deprecated v8::ArrayBuffer:detach API (denoland#1121)
Browse files Browse the repository at this point in the history
  • Loading branch information
bartlomieju committed Nov 12, 2022
1 parent 84880f4 commit 9d15050
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 9 deletions.
17 changes: 13 additions & 4 deletions src/array_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::ptr::NonNull;
use std::slice;

use crate::support::long;
use crate::support::MaybeBool;
use crate::support::Opaque;
use crate::support::Shared;
use crate::support::SharedPtrBase;
Expand All @@ -19,6 +20,7 @@ use crate::ArrayBuffer;
use crate::HandleScope;
use crate::Isolate;
use crate::Local;
use crate::Value;

extern "C" {
fn v8__ArrayBuffer__Allocator__NewDefaultAllocator() -> *mut Allocator;
Expand All @@ -35,7 +37,10 @@ extern "C" {
isolate: *mut Isolate,
backing_store: *const SharedRef<BackingStore>,
) -> *const ArrayBuffer;
fn v8__ArrayBuffer__Detach(this: *const ArrayBuffer);
fn v8__ArrayBuffer__Detach(
this: *const ArrayBuffer,
key: *const Value,
) -> MaybeBool;
fn v8__ArrayBuffer__Data(this: *const ArrayBuffer) -> *mut c_void;
fn v8__ArrayBuffer__IsDetachable(this: *const ArrayBuffer) -> bool;
fn v8__ArrayBuffer__WasDetached(this: *const ArrayBuffer) -> bool;
Expand Down Expand Up @@ -402,13 +407,17 @@ impl ArrayBuffer {
/// Detaches this ArrayBuffer and all its views (typed arrays).
/// Detaching sets the byte length of the buffer and all typed arrays to zero,
/// preventing JavaScript from ever accessing underlying backing store.
/// ArrayBuffer should have been externalized and must be detachable.
/// ArrayBuffer should have been externalized and must be detachable. Returns
/// `None` if the key didn't pass the [[ArrayBufferDetachKey]] check,
/// and `Some(true)` otherwise.
#[inline(always)]
pub fn detach(&self) {
pub fn detach(&self, key: Local<Value>) -> Option<bool> {
// V8 terminates when the ArrayBuffer is not detachable. Non-detachable
// buffers are buffers that are in use by WebAssembly or asm.js.
if self.is_detachable() {
unsafe { v8__ArrayBuffer__Detach(self) }
unsafe { v8__ArrayBuffer__Detach(self, &*key) }.into()
} else {
Some(true)
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -820,8 +820,10 @@ void* v8__ArrayBuffer__Data(const v8::ArrayBuffer& self) {
return ptr_to_local(&self)->Data();
}

void v8__ArrayBuffer__Detach(const v8::ArrayBuffer& self) {
ptr_to_local(&self)->Detach();
MaybeBool v8__ArrayBuffer__Detach(const v8::ArrayBuffer& self,
const v8::Value* key) {
return maybe_to_maybe_bool(ptr_to_local(&self)->Detach(
ptr_to_local(key)));
}

bool v8__ArrayBuffer__IsDetachable(const v8::ArrayBuffer& self) {
Expand Down
7 changes: 4 additions & 3 deletions tests/test_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,15 +576,16 @@ fn array_buffer() {
assert!(!ab.was_detached());
assert!(ab.is_detachable());

ab.detach();
let key = v8::undefined(scope);
assert!(ab.detach(key.into()).unwrap());
assert_eq!(0, ab.byte_length());
assert!(ab.was_detached());
ab.detach(); // Calling it twice should be a no-op.
assert!(ab.detach(key.into()).unwrap()); // Calling it twice should be a no-op.

// detecting if it was detached on a zero-length ArrayBuffer should work
let empty_ab = v8::ArrayBuffer::new(scope, 0);
assert!(!empty_ab.was_detached());
empty_ab.detach();
assert!(empty_ab.detach(key.into()).unwrap());
assert!(empty_ab.was_detached());

let bs = v8::ArrayBuffer::new_backing_store(scope, 84);
Expand Down

0 comments on commit 9d15050

Please sign in to comment.