Merge pull request #1058 from neon-bindings/kv/borrow-fix

fix(neon): Fix a panic when borrowing an empty buffer or typed array

.github/workflows/ci.yml

@@ -25,9 +25,9 @@ jobs:
       - name: Set Matrix
         id: set_matrix
         env:
-          FULL_NODE_VERSIONS: '["18.x", "20.x", "22.x"]'
+          FULL_NODE_VERSIONS: '["18.x", "20.x"]'
           FULL_RUST_TOOLCHAINS: '["stable", "nightly"]'
-          PARTIAL_NODE_VERSIONS: '["22.x"]'
+          PARTIAL_NODE_VERSIONS: '["20.x"]'
           PARTIAL_RUST_TOOLCHAINS: '["stable"]'
           HAS_FULL_MATRIX_LABEL: ${{ contains(github.event.pull_request.labels.*.name, 'full matrix') }}
           IS_PUSHED: ${{ github.event_name == 'push' }}

.github/workflows/lint.yml

@@ -20,7 +20,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [22.x]
+        node-version: [20.x]
         rust-toolchain: [nightly]
 
     steps:

crates/neon/src/types_impl/buffer/lock.rs

@@ -58,7 +58,9 @@ impl Ledger {
         ledger: &'a RefCell<Self>,
         data: &'a [T],
     ) -> Result<Ref<'a, T>, BorrowError> {
-        ledger.borrow_mut().try_add_borrow(data)?;
+        if !data.is_empty() {
+            ledger.borrow_mut().try_add_borrow(data)?;
+        }
 
         Ok(Ref { ledger, data })
     }
@@ -69,7 +71,9 @@ impl Ledger {
         ledger: &'a RefCell<Self>,
         data: &'a mut [T],
     ) -> Result<RefMut<'a, T>, BorrowError> {
-        ledger.borrow_mut().try_add_borrow_mut(data)?;
+        if !data.is_empty() {
+            ledger.borrow_mut().try_add_borrow_mut(data)?;
+        }
 
         Ok(RefMut { ledger, data })
     }

crates/neon/src/types_impl/buffer/mod.rs

@@ -138,6 +138,10 @@ impl<'a, T> DerefMut for RefMut<'a, T> {
 
 impl<'a, T> Drop for Ref<'a, T> {
     fn drop(&mut self) {
+        if self.is_empty() {
+            return;
+        }
+
         let mut ledger = self.ledger.borrow_mut();
         let range = Ledger::slice_to_range(self.data);
         let i = ledger.shared.iter().rposition(|r| r == &range).unwrap();
@@ -148,6 +152,10 @@ impl<'a, T> Drop for Ref<'a, T> {
 
 impl<'a, T> Drop for RefMut<'a, T> {
     fn drop(&mut self) {
+        if self.is_empty() {
+            return;
+        }
+
         let mut ledger = self.ledger.borrow_mut();
         let range = Ledger::slice_to_range(self.data);
         let i = ledger.owned.iter().rposition(|r| r == &range).unwrap();

crates/neon/src/types_impl/buffer/types.rs

@@ -5,7 +5,7 @@ use crate::{
     handle::{internal::TransparentNoCopyWrapper, Handle},
     object::Object,
     result::{JsResult, Throw},
-    sys::{self, raw, TypedArrayType},
+    sys::{self, raw, typedarray::TypedArrayInfo, TypedArrayType},
     types_impl::{
         buffer::{
             lock::{Ledger, Lock},
@@ -549,7 +549,7 @@ where
             let value = self.to_local();
             let info = sys::typedarray::info(env, value);
 
-            slice::from_raw_parts(info.data.cast(), info.length)
+            slice_from_info(info)
         }
     }
 
@@ -562,7 +562,7 @@ where
             let value = self.to_local();
             let info = sys::typedarray::info(env, value);
 
-            slice::from_raw_parts_mut(info.data.cast(), info.length)
+            slice_from_info_mut(info)
         }
     }
 
@@ -579,10 +579,7 @@ where
             let info = sys::typedarray::info(env, value);
 
             // The borrowed data must be guarded by `Ledger` before returning
-            Ledger::try_borrow(
-                &lock.ledger,
-                slice::from_raw_parts(info.data.cast(), info.length),
-            )
+            Ledger::try_borrow(&lock.ledger, slice_from_info(info))
         }
     }
 
@@ -599,10 +596,7 @@ where
             let info = sys::typedarray::info(env, value);
 
             // The borrowed data must be guarded by `Ledger` before returning
-            Ledger::try_borrow_mut(
-                &lock.ledger,
-                slice::from_raw_parts_mut(info.data.cast(), info.length),
-            )
+            Ledger::try_borrow_mut(&lock.ledger, slice_from_info_mut(info))
         }
     }
 
@@ -778,6 +772,22 @@ where
     }
 }
 
+unsafe fn slice_from_info<'a, T>(info: TypedArrayInfo) -> &'a [T] {
+    if info.length == 0 {
+        &[]
+    } else {
+        slice::from_raw_parts(info.data.cast(), info.length)
+    }
+}
+
+unsafe fn slice_from_info_mut<'a, T>(info: TypedArrayInfo) -> &'a mut [T] {
+    if info.length == 0 {
+        &mut []
+    } else {
+        slice::from_raw_parts_mut(info.data.cast(), info.length)
+    }
+}
+
 macro_rules! impl_typed_array {
     ($typ:ident, $etyp:ty, $($pattern:pat_param)|+, $tag:ident, $alias:ident, $two:expr$(,)?) => {
         impl private::Sealed for $etyp {}

test/napi/lib/typedarrays.js

@@ -414,6 +414,20 @@ describe("Typed arrays", function () {
     assert.equal(b[3], 55);
   });
 
+  it("copies from a source buffer to a destination with borrow API", function () {
+    for (const f of [addon.copy_buffer, addon.copy_buffer_with_borrow]) {
+      const a = Buffer.from([1, 2, 3]);
+      const b = Buffer.from([0, 0, 0, 4, 5, 6]);
+
+      // Full
+      addon.copy_buffer(a, b);
+      assert.deepEqual([...b], [1, 2, 3, 4, 5, 6]);
+
+      // Empty buffers
+      addon.copy_buffer(Buffer.alloc(0), Buffer.alloc(0));
+    }
+  });
+
   it("zeroes the byteLength when an ArrayBuffer is detached", function () {
     var buf = new ArrayBuffer(16);
     assert.strictEqual(buf.byteLength, 16);

test/napi/src/js/typedarrays.rs

@@ -379,3 +379,32 @@ pub fn write_buffer_with_borrow_mut(mut cx: FunctionContext) -> JsResult<JsUndef
 
     Ok(cx.undefined())
 }
+
+pub fn copy_buffer(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let a = cx.argument::<JsTypedArray<u8>>(0)?;
+    let mut b = cx.argument::<JsTypedArray<u8>>(1)?;
+    let a_len = a.as_slice(&cx).len();
+    let b_len = b.as_slice(&cx).len();
+    let n = a_len.min(b_len);
+    let a = a.as_slice(&cx)[..n].to_vec();
+
+    b.as_mut_slice(&mut cx)[..n].copy_from_slice(&a);
+
+    Ok(cx.undefined())
+}
+
+pub fn copy_buffer_with_borrow(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let a = cx.argument::<JsTypedArray<u8>>(0)?;
+    let mut b = cx.argument::<JsTypedArray<u8>>(1)?;
+    let lock = cx.lock();
+    let (Ok(a), Ok(mut b)) = (a.try_borrow(&lock), b.try_borrow_mut(&lock)) else {
+        return cx.throw_error("Borrow Error");
+    };
+
+    let n = a.len().min(b.len());
+
+    b[..n].copy_from_slice(&a);
+    drop((a, b));
+
+    Ok(cx.undefined())
+}

test/napi/src/lib.rs

@@ -285,6 +285,8 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> {
     cx.export_function("read_buffer_with_borrow", read_buffer_with_borrow)?;
     cx.export_function("write_buffer_with_lock", write_buffer_with_lock)?;
     cx.export_function("write_buffer_with_borrow_mut", write_buffer_with_borrow_mut)?;
+    cx.export_function("copy_buffer", copy_buffer)?;
+    cx.export_function("copy_buffer_with_borrow", copy_buffer_with_borrow)?;
     cx.export_function("byte_length", byte_length)?;
     cx.export_function("call_nullary_method", call_nullary_method)?;
     cx.export_function("call_unary_method", call_unary_method)?;