Skip to content

Commit 112e773

Browse files
committed
neon: Relax the lifetime constraints on TypedArray borrows
The current lifetimes are overly restrictive. The reference is valid for as long as the handle is valid. Since the `Context` is always the most narrow scope, it will be valid for at *least* as long as `Context`, even if the `Handle` type is dropped. This allows downcasting and returning a borrow within the same function.
1 parent 5cf493e commit 112e773

File tree

5 files changed

+87
-49
lines changed

5 files changed

+87
-49
lines changed

src/types/buffer/mod.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,43 +22,40 @@ pub trait TypedArray: private::Sealed {
2222
///
2323
/// This may not be used if a mutable borrow is in scope. For the dynamically
2424
/// checked variant see [`TypedArray::try_borrow`].
25-
fn as_slice<'a: 'b, 'b, C>(&'b self, cx: &'b C) -> &'b [Self::Item]
25+
fn as_slice<'cx, 'a, C>(&self, cx: &'a C) -> &'a [Self::Item]
2626
where
27-
C: Context<'a>;
27+
C: Context<'cx>;
2828

2929
/// Statically checked mutable borrow of binary data.
3030
///
3131
/// This may not be used if any other borrow is in scope. For the dynamically
3232
/// checked variant see [`TypedArray::try_borrow_mut`].
33-
fn as_mut_slice<'a: 'b, 'b, C>(&'b mut self, cx: &'b mut C) -> &'b mut [Self::Item]
33+
fn as_mut_slice<'cx, 'a, C>(&mut self, cx: &'a mut C) -> &'a mut [Self::Item]
3434
where
35-
C: Context<'a>;
35+
C: Context<'cx>;
3636

3737
/// Dynamically checked immutable borrow of binary data, returning an error if the
3838
/// the borrow would overlap with a mutable borrow.
3939
///
4040
/// The borrow lasts until [`Ref`] exits scope.
4141
///
4242
/// This is the dynamically checked version of [`TypedArray::as_slice`].
43-
fn try_borrow<'a: 'b, 'b, C>(
44-
&self,
45-
lock: &'b Lock<'b, C>,
46-
) -> Result<Ref<'b, Self::Item>, BorrowError>
43+
fn try_borrow<'cx, 'a, C>(&self, lock: &'a Lock<C>) -> Result<Ref<'a, Self::Item>, BorrowError>
4744
where
48-
C: Context<'a>;
45+
C: Context<'cx>;
4946

5047
/// Dynamically checked mutable borrow of binary data, returning an error if the
5148
/// the borrow would overlap with an active borrow.
5249
///
5350
/// The borrow lasts until [`RefMut`] exits scope.
5451
///
5552
/// This is the dynamically checked version of [`TypedArray::as_mut_slice`].
56-
fn try_borrow_mut<'a: 'b, 'b, C>(
53+
fn try_borrow_mut<'cx, 'a, C>(
5754
&mut self,
58-
lock: &'b Lock<'b, C>,
59-
) -> Result<RefMut<'b, Self::Item>, BorrowError>
55+
lock: &'a Lock<C>,
56+
) -> Result<RefMut<'a, Self::Item>, BorrowError>
6057
where
61-
C: Context<'a>;
58+
C: Context<'cx>;
6259
}
6360

6461
#[derive(Debug)]

src/types/buffer/types.rs

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -89,39 +89,44 @@ impl private::Sealed for JsBuffer {}
8989
impl TypedArray for JsBuffer {
9090
type Item = u8;
9191

92-
fn as_slice<'a: 'b, 'b, C>(&'b self, cx: &'b C) -> &'b [Self::Item]
92+
fn as_slice<'cx, 'a, C>(&self, cx: &'a C) -> &'a [Self::Item]
9393
where
94-
C: Context<'a>,
94+
C: Context<'cx>,
9595
{
96+
// # Safety
97+
// Only the `Context` with the *most* narrow scope is accessible because `compute_scoped`
98+
// and `execute_scope` take an exclusive reference to `Context`. A handle is always
99+
// associated with a `Context` and the value will not be garbage collected while that
100+
// `Context` is in scope. This means that the referenced data is valid *at least* as long
101+
// as `Context`, even if the `Handle` is dropped.
96102
unsafe { neon_runtime::buffer::as_mut_slice(cx.env().to_raw(), self.to_raw()) }
97103
}
98104

99-
fn as_mut_slice<'a: 'b, 'b, C>(&'b mut self, cx: &'b mut C) -> &'b mut [Self::Item]
105+
fn as_mut_slice<'cx, 'a, C>(&mut self, cx: &'a mut C) -> &'a mut [Self::Item]
100106
where
101-
C: Context<'a>,
107+
C: Context<'cx>,
102108
{
109+
// # Safety
110+
// See `as_slice`
103111
unsafe { neon_runtime::buffer::as_mut_slice(cx.env().to_raw(), self.to_raw()) }
104112
}
105113

106-
fn try_borrow<'a: 'b, 'b, C>(
107-
&self,
108-
lock: &'b Lock<'b, C>,
109-
) -> Result<Ref<'b, Self::Item>, BorrowError>
114+
fn try_borrow<'cx, 'a, C>(&self, lock: &'a Lock<C>) -> Result<Ref<'a, Self::Item>, BorrowError>
110115
where
111-
C: Context<'a>,
116+
C: Context<'cx>,
112117
{
113118
// The borrowed data must be guarded by `Ledger` before returning
114119
Ledger::try_borrow(&lock.ledger, unsafe {
115120
neon_runtime::buffer::as_mut_slice(lock.cx.env().to_raw(), self.to_raw())
116121
})
117122
}
118123

119-
fn try_borrow_mut<'a: 'b, 'b, C>(
124+
fn try_borrow_mut<'cx, 'a, C>(
120125
&mut self,
121-
lock: &'b Lock<'b, C>,
122-
) -> Result<RefMut<'b, Self::Item>, BorrowError>
126+
lock: &'a Lock<C>,
127+
) -> Result<RefMut<'a, Self::Item>, BorrowError>
123128
where
124-
C: Context<'a>,
129+
C: Context<'cx>,
125130
{
126131
// The borrowed data must be guarded by `Ledger` before returning
127132
Ledger::try_borrow_mut(&lock.ledger, unsafe {
@@ -197,39 +202,36 @@ impl private::Sealed for JsArrayBuffer {}
197202
impl TypedArray for JsArrayBuffer {
198203
type Item = u8;
199204

200-
fn as_slice<'a: 'b, 'b, C>(&'b self, cx: &'b C) -> &'b [Self::Item]
205+
fn as_slice<'cx, 'a, C>(&self, cx: &'a C) -> &'a [Self::Item]
201206
where
202-
C: Context<'a>,
207+
C: Context<'cx>,
203208
{
204209
unsafe { neon_runtime::arraybuffer::as_mut_slice(cx.env().to_raw(), self.to_raw()) }
205210
}
206211

207-
fn as_mut_slice<'a: 'b, 'b, C>(&'b mut self, cx: &'b mut C) -> &'b mut [Self::Item]
212+
fn as_mut_slice<'cx, 'a, C>(&mut self, cx: &'a mut C) -> &'a mut [Self::Item]
208213
where
209-
C: Context<'a>,
214+
C: Context<'cx>,
210215
{
211216
unsafe { neon_runtime::arraybuffer::as_mut_slice(cx.env().to_raw(), self.to_raw()) }
212217
}
213218

214-
fn try_borrow<'a: 'b, 'b, C>(
215-
&self,
216-
lock: &'b Lock<'b, C>,
217-
) -> Result<Ref<'b, Self::Item>, BorrowError>
219+
fn try_borrow<'cx, 'a, C>(&self, lock: &'a Lock<C>) -> Result<Ref<'a, Self::Item>, BorrowError>
218220
where
219-
C: Context<'a>,
221+
C: Context<'cx>,
220222
{
221223
// The borrowed data must be guarded by `Ledger` before returning
222224
Ledger::try_borrow(&lock.ledger, unsafe {
223225
neon_runtime::arraybuffer::as_mut_slice(lock.cx.env().to_raw(), self.to_raw())
224226
})
225227
}
226228

227-
fn try_borrow_mut<'a: 'b, 'b, C>(
229+
fn try_borrow_mut<'cx, 'a, C>(
228230
&mut self,
229-
lock: &'b Lock<'b, C>,
230-
) -> Result<RefMut<'b, Self::Item>, BorrowError>
231+
lock: &'a Lock<C>,
232+
) -> Result<RefMut<'a, Self::Item>, BorrowError>
231233
where
232-
C: Context<'a>,
234+
C: Context<'cx>,
233235
{
234236
// The borrowed data must be guarded by `Ledger` before returning
235237
Ledger::try_borrow_mut(&lock.ledger, unsafe {
@@ -272,9 +274,9 @@ impl<T> Managed for JsTypedArray<T> {
272274
impl<T: Copy> TypedArray for JsTypedArray<T> {
273275
type Item = T;
274276

275-
fn as_slice<'a: 'b, 'b, C>(&'b self, cx: &'b C) -> &'b [Self::Item]
277+
fn as_slice<'cx, 'a, C>(&self, cx: &'a C) -> &'a [Self::Item]
276278
where
277-
C: Context<'a>,
279+
C: Context<'cx>,
278280
{
279281
unsafe {
280282
let env = cx.env().to_raw();
@@ -285,9 +287,9 @@ impl<T: Copy> TypedArray for JsTypedArray<T> {
285287
}
286288
}
287289

288-
fn as_mut_slice<'a: 'b, 'b, C>(&'b mut self, cx: &'b mut C) -> &'b mut [Self::Item]
290+
fn as_mut_slice<'cx, 'a, C>(&mut self, cx: &'a mut C) -> &'a mut [Self::Item]
289291
where
290-
C: Context<'a>,
292+
C: Context<'cx>,
291293
{
292294
unsafe {
293295
let env = cx.env().to_raw();
@@ -298,12 +300,12 @@ impl<T: Copy> TypedArray for JsTypedArray<T> {
298300
}
299301
}
300302

301-
fn try_borrow<'a: 'b, 'b, C>(
303+
fn try_borrow<'cx, 'b, C>(
302304
&self,
303305
lock: &'b Lock<'b, C>,
304306
) -> Result<Ref<'b, Self::Item>, BorrowError>
305307
where
306-
C: Context<'a>,
308+
C: Context<'cx>,
307309
{
308310
unsafe {
309311
let env = lock.cx.env().to_raw();
@@ -318,12 +320,12 @@ impl<T: Copy> TypedArray for JsTypedArray<T> {
318320
}
319321
}
320322

321-
fn try_borrow_mut<'a: 'b, 'b, C>(
323+
fn try_borrow_mut<'cx, 'a, C>(
322324
&mut self,
323-
lock: &'b Lock<'b, C>,
324-
) -> Result<RefMut<'b, Self::Item>, BorrowError>
325+
lock: &'a Lock<'a, C>,
326+
) -> Result<RefMut<'a, Self::Item>, BorrowError>
325327
where
326-
C: Context<'a>,
328+
C: Context<'cx>,
327329
{
328330
unsafe {
329331
let env = lock.cx.env().to_raw();

test/napi/lib/objects.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,4 +233,12 @@ describe("JsObject", function () {
233233
);
234234
assert.equal(addon.get_own_property_names(object).length, 1);
235235
});
236+
237+
it("data borrowed on the heap can be held longer than the handle", function () {
238+
const msg = "Hello, World!";
239+
const buf = Buffer.from(msg);
240+
241+
assert.strictEqual(addon.byte_length(msg), buf.length);
242+
assert.strictEqual(addon.byte_length(buf), buf.length);
243+
});
236244
});

test/napi/src/js/objects.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::borrow::Cow;
2+
13
use neon::prelude::*;
24
use neon::types::buffer::TypedArray;
35

@@ -189,3 +191,31 @@ pub fn write_buffer_with_borrow_mut(mut cx: FunctionContext) -> JsResult<JsUndef
189191

190192
Ok(cx.undefined())
191193
}
194+
195+
// Accepts either a `JsString` or `JsBuffer` and returns the contents as
196+
// as bytes; avoids copying.
197+
fn get_bytes<'cx, 'a, C>(cx: &'a mut C, v: Handle<JsValue>) -> NeonResult<Cow<'a, [u8]>>
198+
where
199+
C: Context<'cx>,
200+
{
201+
if let Ok(v) = v.downcast::<JsString, _>(cx) {
202+
return Ok(Cow::Owned(v.value(cx).into_bytes()));
203+
}
204+
205+
if let Ok(v) = v.downcast::<JsBuffer, _>(cx) {
206+
return Ok(Cow::Borrowed(v.as_slice(cx)));
207+
}
208+
209+
cx.throw_type_error("Value must be a string or Buffer")
210+
}
211+
212+
pub fn byte_length(mut cx: FunctionContext) -> JsResult<JsNumber> {
213+
let v = cx.argument::<JsValue>(0)?;
214+
let bytes = get_bytes(&mut cx, v)?;
215+
216+
// `v` is dropped here, but `bytes` is still valid since the data is on the V8 heap
217+
218+
let len = bytes.len();
219+
220+
Ok(cx.number(len as f64))
221+
}

test/napi/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> {
242242
cx.export_function("read_buffer_with_borrow", read_buffer_with_borrow)?;
243243
cx.export_function("write_buffer_with_lock", write_buffer_with_lock)?;
244244
cx.export_function("write_buffer_with_borrow_mut", write_buffer_with_borrow_mut)?;
245+
cx.export_function("byte_length", byte_length)?;
245246

246247
cx.export_function("create_date", create_date)?;
247248
cx.export_function("get_date_value", get_date_value)?;

0 commit comments

Comments
 (0)