From 1104742983f7f113045615cea792bc96310c01a1 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 28 Feb 2022 01:26:48 +0800 Subject: [PATCH] fix(napi): Buffer value lifetime should align the Rust lifetime --- .../src/bindgen_runtime/js_values/buffer.rs | 41 +++++++++++++++++- examples/napi/__test__/typegen.spec.ts.md | 1 + examples/napi/__test__/typegen.spec.ts.snap | Bin 2681 -> 2702 bytes examples/napi/__test__/values.spec.ts | 7 +++ examples/napi/index.d.ts | 1 + examples/napi/src/typed_array.rs | 5 +++ 6 files changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/js_values/buffer.rs b/crates/napi/src/bindgen_runtime/js_values/buffer.rs index d1c7a1bc..db5233cd 100644 --- a/crates/napi/src/bindgen_runtime/js_values/buffer.rs +++ b/crates/napi/src/bindgen_runtime/js_values/buffer.rs @@ -3,15 +3,33 @@ use std::{mem, ptr}; use crate::{bindgen_prelude::*, check_status, sys, Result, ValueType}; -/// zero copy u8 vector shared between rust and napi +/// Zero copy u8 vector shared between rust and napi. +/// Auto reference the raw JavaScript value, and release it when dropped. +/// So it is safe to use it in `async fn`, the `&[u8]` under the hood will not be dropped until the `drop` called. pub struct Buffer { inner: mem::ManuallyDrop>, + raw: Option<(sys::napi_ref, sys::napi_env)>, } +impl Drop for Buffer { + fn drop(&mut self) { + if let Some((ref_, env)) = self.raw { + check_status_or_throw!( + env, + unsafe { sys::napi_delete_reference(env, ref_) }, + "Failed to delete Buffer reference in drop" + ); + } + } +} + +unsafe impl Send for Buffer {} + impl From> for Buffer { fn from(data: Vec) -> Self { Buffer { inner: mem::ManuallyDrop::new(data), + raw: None, } } } @@ -68,7 +86,11 @@ impl FromNapiValue for Buffer { unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> Result { let mut buf = ptr::null_mut(); let mut len = 0; - + let mut ref_ = ptr::null_mut(); + check_status!( + unsafe { sys::napi_create_reference(env, napi_val, 1, &mut ref_) }, + "Failed to create reference from Buffer" + )?; check_status!( unsafe { sys::napi_get_buffer_info(env, napi_val, &mut buf, &mut len as *mut usize) }, "Failed to convert napi buffer into rust Vec" @@ -76,12 +98,27 @@ impl FromNapiValue for Buffer { Ok(Self { inner: mem::ManuallyDrop::new(unsafe { Vec::from_raw_parts(buf as *mut _, len, len) }), + raw: Some((ref_, env)), }) } } impl ToNapiValue for Buffer { unsafe fn to_napi_value(env: sys::napi_env, mut val: Self) -> Result { + // From Node.js value, not from `Vec` + if let Some((ref_, _)) = val.raw { + let mut buf = ptr::null_mut(); + check_status!( + unsafe { sys::napi_get_reference_value(env, ref_, &mut buf) }, + "Failed to get Buffer value from reference" + )?; + check_status!( + unsafe { sys::napi_delete_reference(env, ref_) }, + "Failed to delete Buffer reference" + )?; + val.raw = None; // Prevent double free + return Ok(buf); + } let len = val.inner.len(); let mut ret = ptr::null_mut(); check_status!( diff --git a/examples/napi/__test__/typegen.spec.ts.md b/examples/napi/__test__/typegen.spec.ts.md index 4e6eb413..f7f6738f 100644 --- a/examples/napi/__test__/typegen.spec.ts.md +++ b/examples/napi/__test__/typegen.spec.ts.md @@ -152,6 +152,7 @@ Generated by [AVA](https://avajs.dev). export function createExternalTypedArray(): Uint32Array␊ export function mutateTypedArray(input: Float32Array): void␊ export function derefUint8Array(a: Uint8Array, b: Uint8ClampedArray): number␊ + export function bufferPassThrough(buf: Buffer): Promise␊ /**␊ * \`constructor\` option for \`struct\` requires all fields to be public,␊ * otherwise tag impl fn as constructor␊ diff --git a/examples/napi/__test__/typegen.spec.ts.snap b/examples/napi/__test__/typegen.spec.ts.snap index 335738c18ebe44ba50ae103449c3727e1fbb1f1a..39d0b36c51ab03d6d4ad56ba94c85075245ed116 100644 GIT binary patch delta 2124 zcmV-S2($P36pj^tK~_N^Q*L2!b7*gLAa*kf0{{b;BTW5AfHgT;Si00PnnH#>+kkA% zqLX=boESg*rh*@f2mk;800003tr^>H+sHL-Q55i7|6uB(SSnBwUxFYN5(lzuXYItY zVawetyvAloj--jH8HO{ot$Gb~|Dex(DRzINf6(vgC-j_u8Il_DBH2L#2jUFRea?0G z%g~QxaH)R&m1(XccJmiCmT44{5fdz-nvp;z2@5pQ)0mNv4aty4j4MH7fYj>eU&4@T z`u*yEfA{DQkMQ4be*fr?zx~hZv&TfMhzs2bxuSl=$m3@#u*uBk6K4~5X{zOuoM%Y< zDd~5Q_n(}9oEiXj6U#)CAfif<-5bpkL8Bx86Ug;yutjcH2uTi#gXo0oDeB|{Oxsp5t0{BxwmofhAibe`;0;}~m`I7tu{tdCo1;oxzVCX2i zCy!H26Wzq6hOA}$Kt&9b1==j-V^$kqLER%r2W2T9L>7Y=QXeO;$t}5pz(v}x0m+ki z0%(6$!o;5(4Q+V*Dj2IOa_A_HZQa7f!s&Z@6`1w+P@C0-hDIG9=#d=tw z3(HyJof)wMWdQbMV$HLZ-eIc0m?VZOoH&1l8U&7zPRJFFQg9QRU{fPBxR-=Fzqz5T z*N`EUC9+m2_XhVJ7oo8#Qr)J>rK#A2?8p%W{8~GE^A5aSL3Mz_Z{PQ*hWG3F`vFyY znqj8FC1#GS>G)ozzz;hwA5z050bM{W7DGseWC#fX&Oos1Lh@8ueV*3K+e?*}*cE^5 z;tB;n5-gVlDzeF0_&81&?r-8^Po@bxKgZ`&egls$@Nt)q#;C(26+M9AfW|5)5G-__XmGZp{-oM4}6mA zeNS-|M7z0W+ZPxW_R1~4dTXvfHbN~X9&C1VR|dO=_?4;d`moyrJ0~)VAW9pG?vuF& zXMdd^E3j^bmR4(yPDAVOz-PSoB1s+QwZ;fp!iGrvrA4Kdm0cNsHA@(gCpr1%s^lBf z9K$q?PLz(uCDWC!e4%WtkjLbs>3p!^kjcmFp3Rh{k9PMXr0LHo$Ez`Lb7*i`1kNJirHehu^?D~<@C!~lwwl7Bel5$@0Dc>m@(yRC~AcL+8g3A%z$XjDw> zW_*XTM(wt#XvOD&OL*BaAVn>-g%*pT!^fgI8{Q1FJy%m(B|?-y83u$YnJ`FKp{1nY z+<9Pf4w7)Fp|2T90Sa3WGs3l;+N<)-8m{SU#d*qu`bq}`DtqfX1Uspa-9xU0E;F`qtyF-^cM z`1J!XIf^A{_c?{;fGLZm$5fN-vlRFYQvn5M&Ob~=(~brE;r!zE?%5BW)zv};JEk3> zXPH|flnKPEd4p}By{70n=ZpD*Cx79r^RUi^mp(OZlL!>+gO+Vqd76M2av6m~EsKn-5@(T&fwj+vY;iTrxCa9K9> zF4?q`Glq4mQ{q}HApp+AgGDV*1iB?9yJ1mgH+E2&(5cV*jNt|E?2fXg-G6kA+L~8k z2Z~YFycJDM#sfK+P<8zOt}ex(|MSH+sHL-Q55jo{=w8mu~eWWz9C43Tp-JK)=n%N zw%pCaYix$(NSc_MVK_tEs@FjGrO$otOR@Wf{e+$~LsCP3k(BHpfdg@d=RW5;{B`I@ zGPqQ~{Khob5xe=T8p|{a$%qM-P|ZjnlY|AD=xNMI$cALdBgU1WF+ghd%dcTbHT_}r zUw?S?r$_kjcYl2J&F`;QpFJj8MO^4k$QAV?Mjk&~flX#MpE#SiOH(bMN_uzWugiHs-cc!9S;r zsOiL)k$HXou9GbBUMK2wGsRFdupKnp{a90s;J~pvxG4c10tFHi6apn|w)r2LFcGG zHiwBnIUYnHONKOHW>=Z4uh7*pFhZCJ-`GWs(IDH;3&6M@v&6>vQBqd9Fn*E78nm#s z-Zi9hb#R{EGFkLKl{^ z#5*%$2g(5K$;6sxDZRr~e=$i6RXA~f3N;8EA)Syb8l~VSG{L4uXmBqHb$)Y0S+5~O zC`)9mQtl1zJ1#7*j_U0XUyMpQfh2OsKQ4R0c^Y;U)^fbdv zgGCJ1_$}wg3nCzH-W0X9E33KF>g;WBZYjwe3pw z&cO;<4E;3|0cYw>C3nLl@wz@(Lw2?EMW}V|NJB?Aw79dZJ09nCAM0W~TbFQ~M|+qw z^eK*aClVwCnApP$Ff&E9I<*rN_7;>2e0!TKSYN71}nVg;@>$dAC1Qde?gS>!l zkkA63>EZhG`-8uy&{nSB2R_O5zNa_}qTO7x?F)r5Gn+KDr z26}%jj}=(ALQAVvqSMg&JMbCry+~4rd95)*maribe_>InWo1{!U(FImundGtb#=U97l6ths;W?+QMlQ896w@f*q#wcDm571#Th@Umk- zidtq1B^EJj$z8G6q7 zV!ZPtoOKS?x$x4arVVmEqkJxN_(*^AVFZjD^q_ERrza#v$ctJb6W2|WxU8ADVe4*(R9*IDWW%6ffMa+$8 { @@ -382,6 +383,12 @@ test('async move', async (t) => { t.is(await asyncMultiTwo(2), 4) }) +test('buffer passthrough', async (t) => { + const fixture = Buffer.from('hello world') + const ret = await bufferPassThrough(fixture) + t.deepEqual(ret, fixture) +}) + test('either', (t) => { t.is(eitherStringOrNumber(2), 2) t.is(eitherStringOrNumber('hello'), 'hello'.length) diff --git a/examples/napi/index.d.ts b/examples/napi/index.d.ts index 2a6ae7e7..9a636c1b 100644 --- a/examples/napi/index.d.ts +++ b/examples/napi/index.d.ts @@ -142,6 +142,7 @@ export function convertU32Array(input: Uint32Array): Array export function createExternalTypedArray(): Uint32Array export function mutateTypedArray(input: Float32Array): void export function derefUint8Array(a: Uint8Array, b: Uint8ClampedArray): number +export function bufferPassThrough(buf: Buffer): Promise /** * `constructor` option for `struct` requires all fields to be public, * otherwise tag impl fn as constructor diff --git a/examples/napi/src/typed_array.rs b/examples/napi/src/typed_array.rs index c4a769db..26082017 100644 --- a/examples/napi/src/typed_array.rs +++ b/examples/napi/src/typed_array.rs @@ -33,3 +33,8 @@ fn mutate_typed_array(mut input: Float32Array) { fn deref_uint8_array(a: Uint8Array, b: Uint8ClampedArray) -> u32 { (a.len() + b.len()) as u32 } + +#[napi] +async fn buffer_pass_through(buf: Buffer) -> Result { + Ok(buf) +}