From 6a252c70d21e1b20dedf282e10abeed83af3687a Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 6 Apr 2022 21:08:00 +0800 Subject: [PATCH] fix(napi): make buffer Send & Sync safe --- .../src/bindgen_runtime/js_values/buffer.rs | 53 ++++++++++++++---- examples/napi/__test__/typegen.spec.ts.md | 1 + examples/napi/__test__/typegen.spec.ts.snap | Bin 2883 -> 2889 bytes examples/napi/__test__/values.spec.ts | 10 ++++ examples/napi/index.d.ts | 1 + examples/napi/src/typed_array.rs | 25 +++++++++ memory-testing/index.mjs | 2 - 7 files changed, 79 insertions(+), 13 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/js_values/buffer.rs b/crates/napi/src/bindgen_runtime/js_values/buffer.rs index c54347b3..4c3fde3c 100644 --- a/crates/napi/src/bindgen_runtime/js_values/buffer.rs +++ b/crates/napi/src/bindgen_runtime/js_values/buffer.rs @@ -1,13 +1,17 @@ +use std::mem; use std::ops::{Deref, DerefMut}; -use std::{mem, ptr}; +use std::ptr; +use std::slice; use crate::{bindgen_prelude::*, check_status, sys, Result, ValueType}; /// 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. +/// Clone will create a new `Reference` to the same underlying `JavaScript Buffer`. pub struct Buffer { - inner: mem::ManuallyDrop>, + inner: &'static mut [u8], + capacity: usize, raw: Option<(sys::napi_ref, sys::napi_env)>, } @@ -16,7 +20,7 @@ impl Drop for Buffer { if let Some((ref_, env)) = self.raw { check_status_or_throw!( env, - unsafe { sys::napi_delete_reference(env, ref_) }, + unsafe { sys::napi_reference_unref(env, ref_, &mut 0) }, "Failed to delete Buffer reference in drop" ); } @@ -25,10 +29,36 @@ impl Drop for Buffer { unsafe impl Send for Buffer {} +impl Buffer { + pub fn clone(&mut self, env: &Env) -> Result { + if let Some((ref_, _)) = self.raw { + check_status!( + unsafe { sys::napi_reference_ref(env.0, ref_, &mut 0) }, + "Failed to ref Buffer reference in Buffer::clone" + )?; + Ok(Self { + inner: unsafe { slice::from_raw_parts_mut(self.inner.as_mut_ptr(), self.inner.len()) }, + capacity: self.capacity, + raw: Some((ref_, env.0)), + }) + } else { + Err(Error::new( + Status::InvalidArg, + "clone only available when the buffer is created from a JavaScript Buffer".to_owned(), + )) + } + } +} + impl From> for Buffer { - fn from(data: Vec) -> Self { + fn from(mut data: Vec) -> Self { + let inner_ptr = data.as_mut_ptr(); + let len = data.len(); + let capacity = data.capacity(); + mem::forget(data); Buffer { - inner: mem::ManuallyDrop::new(data), + inner: unsafe { slice::from_raw_parts_mut(inner_ptr, len) }, + capacity, raw: None, } } @@ -48,13 +78,13 @@ impl From<&[u8]> for Buffer { impl AsRef<[u8]> for Buffer { fn as_ref(&self) -> &[u8] { - self.inner.as_slice() + self.inner } } impl AsMut<[u8]> for Buffer { fn as_mut(&mut self) -> &mut [u8] { - self.inner.as_mut_slice() + self.inner } } @@ -62,13 +92,13 @@ impl Deref for Buffer { type Target = [u8]; fn deref(&self) -> &Self::Target { - self.inner.as_slice() + self.inner } } impl DerefMut for Buffer { fn deref_mut(&mut self) -> &mut Self::Target { - self.inner.as_mut_slice() + self.inner } } @@ -97,7 +127,8 @@ impl FromNapiValue for Buffer { )?; Ok(Self { - inner: mem::ManuallyDrop::new(unsafe { Vec::from_raw_parts(buf as *mut _, len, len) }), + inner: unsafe { slice::from_raw_parts_mut(buf as *mut _, len) }, + capacity: len, raw: Some((ref_, env)), }) } @@ -128,7 +159,7 @@ impl ToNapiValue for Buffer { len, val.inner.as_mut_ptr() as *mut _, Some(drop_buffer), - Box::into_raw(Box::new((len, val.inner.capacity()))) as *mut _, + Box::into_raw(Box::new((len, val.capacity))) as *mut _, &mut ret, ) }, diff --git a/examples/napi/__test__/typegen.spec.ts.md b/examples/napi/__test__/typegen.spec.ts.md index bd02cfe7..2a93f83a 100644 --- a/examples/napi/__test__/typegen.spec.ts.md +++ b/examples/napi/__test__/typegen.spec.ts.md @@ -172,6 +172,7 @@ Generated by [AVA](https://avajs.dev). export function mutateTypedArray(input: Float32Array): void␊ export function derefUint8Array(a: Uint8Array, b: Uint8ClampedArray): number␊ export function bufferPassThrough(buf: Buffer): Promise␊ + export function asyncReduceBuffer(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 e36dd29b9d2454d119704227b363900a8dcf8616..86e0d3e0fd2d31bb1e8f51be1e188a2137a71e1d 100644 GIT binary patch literal 2889 zcmV-P3%2w@RzVv$~2o*SqKY}0?5(lzu=j_C?VavT7 ze8y!(uB44gF2h~gR(%F~|Dex(Deit@e?n(=x#UVDB{@ytz})4`?9A+Z)4vP{q3|!| z&%ZLoRmg7sD#s!V12SSUOQ~YS7ir3TMbso=Bw#}_M=z0U1n4uXDWr zFWtv6=2vY3C)7?G1` zAVU56{9QX=UbIOcw(eH$fR)FjKjxCa4+Ys{sgMK9fL`zH?sd-I_kZjjo%Y)#&Y}TJ zEyOU3eZ@r#`cvPDG>~4aO*&~xC$FSRc|6+EN4#Z95-B2$GBT%`S)Y^;(Y7*&=s39IvmlBEHA@=1#L;RU&?b8hQgRh{gn zsYqR3zQSdv40wwVm7U8~}% zdRmjj&ljEtPe6B`&_3f=tcw}C(3~OOE{PpT4X`Itqn?HI4m0(|B+*sj#wp|=FoY~3 zS2WDPOlXQlO{~DY1myY64QZ@~3?VI%wMx3zxbJuzXssgEZJJ){j7`Xn7=gpDt>$mu zfwfD>4v_fG`z}@Re%-$BQ>i97X69dF42|n+b!G?0_Qn3@hR52|%*-iRAUa&@;OI4l zIg{CKf#^y859avZOySs6eM}R{lRoN|#mXswW~kZfuBH+w_+j}&6D!TKN11X3TSRCT zZQ<|uz7W1vUtl5}!@DjSA^oIFSC1Cclz$!+XXa@!Mj!f!z#Nzdoim7CFZIq6=jp=B zVlFBO3oH}FJgnS`_?bU7B4OI_f<-yYh3@m{9vJ0q-rpW9jUu1>a08xB;fvCfMg5k_ z@gYsDDo+?RdciAOZ!OJ14>Bq0RhJ=)m3n5AFya&$c?m?DjN3~Kn0FaoQfWArXhAHI z-2J8F_zfk0fuUPUjiI0B$fo28iz(O)v^lpB#YK_V?uv8HFDhs^H)Ci=LQadas_?xA zKY$nbghX)Nz+A`wlFbk*&FZr(UKyHdW+dDl>0X|rp!=Yf=_))fAz z=`hXNy2D&#TSGn&G4*}!#fyQV?p5H>@7P3o&|=#@yD|`{PeAi~bGXsBp%_rD1RTwX zAsOk7I?%u6oiH@lX~UGR8_<5|wI%>UM{M5h*OUi&^;bC&)d-}j zoYdNsE2vZZbl&fGPBrfxTe@vyol^4Q$_!q{0*C@xE-%=NCIF9xz0^oBj8BXVK~4e# z!W6Yt!g5kGtd^%^@bT4*vWo!Js5TePlg^?&MRATP0N5S`$ectYe|?F`ygR1xh?(jb zE%6B2;*=tK%?}>uYPvWsj+;VeDuKEAs@JdvJlLI1O;B@e=6R036`^$}Q)81C;xA z+5n!8)qhUbG!ywd2P=+xHb zgsy&M))Yxqg;u}pVRgm+|I*t7+YS#!z`ioMFJ=)OCT|H_wk)m2*97B>?HseAQ@=J~ zfD=H3Q)inSmBumOTKJ7*x~q4Z(AE^4hR(a%wC;t1I)~RPBVZ{TmfbDdYPr((s+Zl6 zMwWbYRvKv498)RLh_camMs-VlkU3#N=CfMW*W|Pn@|b+ol@k^$BK??8(REt;Xm&qB znE#w{9DIYDK!MD{KoTX$08&&o2)VzCWCYIUu7N&8)e+%Q0-(qfamXXwU(Rv5?m4@y zixqcqs=nH?`%&( zoW1!Xidd}7J|2z*35&!hPX7C!fB#eOBYW3lK_fnOtSgh@9(;dF7~pnCjI=l{SEuH* zZ^7L4bX9-Y^v5&>wcxi+oKY4_knVE^#RZcFOO2@_`A284876%S&R{<~hm*De`{DfJ z_U_pa?S~Hw5lowRgqEdez{nG@SMvtTNHb;ZI`@n5hNpoufYru@Bi))Z%(aZ^dzryU zn)f51&7cd3TRBx}K~;Zv-wx>|+Zf=`hT#y_thdct*Po}6xWpC=Dm_{d}lL6~7hM#ZB zFAz25s;ku2v(?oFjoI}HO~xpsL)e(xA(fY15gIor z4!|+MbTpd2D2g9$9l@(}DU=RgS3| z*SVF!x80~~|K->BJhcVo?TNB`ZT=eThH{sr`Pgnr%>DX3d-8mm$g3s0elNPEMDstd zHyy7(sNYX1vuRu%;wksq|3ax`wkltn)nEirSV@(x|6PAbyT)--M)WrA~po15{b2A!H|b>8p4?H%s+`cCwznHX>W_~Lof zxYz_109xO8St993QbP%V<`epWd>RQ|8fNE%6{6~~20Ke|if%ATzN(z3%WZrg@Db!Y n&t>7RL3bOj?p%R3aMrV65L~a*4+|&pcPsw`itg~%VkZCq95|kD literal 2883 zcmV-J3%v9}RzVI`?&y&~^9C>%-W(_C2> z%1I$Vnjeb@00000000B68ryE$$Te+I6!6>r!8Ap8slZ8m34&Be9LTbrwG+#REqAl< z8k-S0k|riO3}i=s7bSawL+HoF;H!&hXslT!;Q;I0%J* zDS!TzDXv0x^Jh60Ss0KJi&;t)Bfdye<}0Em2_pd;k|7TnmoZHMQpulx2?DC<_h0|# zcMtyX0RR2w_Yc1M?Z3Z%`iLkQ@>sP4F6kg-4%QYp#qjbdp`!=u4Rkn3fCi`=ddk}?{IIGm7gzG>@?nXN45pE5?| zBpQfNzdnE4&X*T$(ub|Pl{*mS5$TV)B=AE)_gE^FzzU$(JG*U;yGN(}Hi@%n zz)}k_%wk`05rh5IcOng>*J_han$pQjsZt(~wls@3Oi3a|#By}$FmXxrvdE!eV7DrO zGK(}Iyq7butwsTrIFMjMq%O22D-dYEJvi)+oW#DDP&EdFX(a&XI293>?B#Yg9M;K1 z5$P}s74KgQkESClh=bdt&w!7>MLI#?(RRLAB?J4|vJVwJA0$?6n~(NmZa(AOcxBq(0NTQfGaMu9kxl!s6hyS=1N} zvi+<8jH@wAO`IR4(<&FnFVa|p7RJ`Qnv_K@aAv49NU+(!6dndbgp9@&_N6f(scTbQ zRk<~3{A?jVcmlffl=d0FVqL7zh2;$Kc1i3&YJfeF8uKiqcbKWqCW)a6Cr+UTfg@xQ zxuRhPZbDORYGMWMC7{l4Zb;)bWC&%6tX0ar#(m4zS4!hkb0XXEB_axto4Blg7Hh|1=NkCPQ znk5ji0U1KRfFp|8b)i@$jk-_j)#+wZ32Yj6afN)2VrB(}f~`{;K8{m{`0P&sP=3;-yGM&@+CLAPGxM|N2}=L~Y!3%#=hKizm) z%tZ}hfn}1Ihn-uIKl7(XBrF?Vu&8Fa(0v}=1EW06`}>2XQPguEPQcS8d{KF_sNYgK zKBS2?NaGtQO|4_Uz=`7k)6z8}CfbBto%tM?a|7&{ii&OCwP9zx&=-Uc|`cnNfD01ohd<&^We0or|g z+W?V{-G5Hj3={b~2PaM*wxNwq1L%04IJIj;?A<}c%0Wg ztcyvxp2BUe!iq_CikB}Z0we^O#KY@peaF#i*WPbnYfpJBJqh!T&&oR|7z#lqr}8(| zaiHK+44P&71Ot4ghU?Fl2Y*Lls#{+MKDBx;DUO6_*L})Vr zc|%u!W!98QRfSf+oMCmx{{Pb31K$o0MZmr=xz8360wx~`TedB&=GO%Ci=7;^q0_%M zU_cN+hEr#o8C|TR>&jrL3cjbaESCnehaS4(g(Bq0n+rRjN`>M zxH&YqEDR)3f()R+oqyUAjhaTZtPVJR;vxX~vdURNuWrW|!9*1~$=8Gs| zu`>I}92*K2iI1H8_h0|~yWU6fuE&B#e2T5BlaU@`cS;$Mc1Mh~IxW|xhTFH`uKIRV zf0*>gGzGKZw@JKXES8|%rwp0{CJmMvQ$_NR#^5td`V^emel`v#Z3Fhh`Ni$s(;wOo z9~LT@G3^LFOW)z5Odwv(8*Bs3En(L=U(6Rg4V(*9?Ob>zThm6lo-utcGx$jJegw1| zbfNGnr>iWe>JR7JA-!Z91H5!$c!_G(+h(o%$J0n$;s}Kuj1EwZmwyuHQ>|kzr%S?q z{3u+OO}tAsX2umi;?Z{CU~Cg*`1bf`N1e^-}c&~bLmHvq?T>Q2{$cYb!whrK!lSn-`6k3Z6W zma337wenqEh{yyl(_;iDO2hvV;`<-JPK9gCu1{z(MkO7>#@!C7yzGk5gh9ao$N1@N zG=0bivLhw_?ZGQz9%hVYUf$Lm3{duX;52JFskx(o%hR%~;n>7b|Eb9_RpYv_GWd3y zbp5~ldY+fIpgcWMPM^(RQ{7bVk~AO7Es42buV*>Wr-`y!vg`MvTWU1_^LP_`{XzVG zMwtzJo1pjdB3k43PE_|Qb+KI`VM=Hnq=ND`E5Pg*z#4C=V4Sc-RNw-hx;Z_Cs;ii} z5j)Y(b#!pQP`NJda9<`k=il6nH#O+gomA)D{+r(6Zm;iTpPC!r%^#mXYr5_=i3Nbx zH(r!TI+N5;0-*VX-lLpGLzjlxd2f}dMy$ck61)XBm?U3S_~~{T-v@jI_0DrexNET8 hhHE=lp$)vNS#XhCZ`1b+Z_@8p{s+o!k_jRw0059jf^Gl+ diff --git a/examples/napi/__test__/values.spec.ts b/examples/napi/__test__/values.spec.ts index a687385a..dd66079c 100644 --- a/examples/napi/__test__/values.spec.ts +++ b/examples/napi/__test__/values.spec.ts @@ -88,6 +88,7 @@ import { chronoDateAdd1Minute, bufferPassThrough, JsRepo, + asyncReduceBuffer, } from '../' test('export const', (t) => { @@ -395,6 +396,15 @@ test('buffer passthrough', async (t) => { t.deepEqual(ret, fixture) }) +test('async reduce buffer', async (t) => { + const input = [1, 2, 3, 4, 5, 6] + const fixture = Buffer.from(input) + t.is( + await asyncReduceBuffer(fixture), + input.reduce((acc, cur) => acc + cur), + ) +}) + 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 eb825895..87b6a8f3 100644 --- a/examples/napi/index.d.ts +++ b/examples/napi/index.d.ts @@ -162,6 +162,7 @@ export function createExternalTypedArray(): Uint32Array export function mutateTypedArray(input: Float32Array): void export function derefUint8Array(a: Uint8Array, b: Uint8ClampedArray): number export function bufferPassThrough(buf: Buffer): Promise +export function asyncReduceBuffer(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 26082017..e2180925 100644 --- a/examples/napi/src/typed_array.rs +++ b/examples/napi/src/typed_array.rs @@ -38,3 +38,28 @@ fn deref_uint8_array(a: Uint8Array, b: Uint8ClampedArray) -> u32 { async fn buffer_pass_through(buf: Buffer) -> Result { Ok(buf) } + +struct AsyncBuffer { + buf: Buffer, +} + +#[napi] +impl Task for AsyncBuffer { + type Output = u32; + type JsValue = u32; + + fn compute(&mut self) -> Result { + Ok(self.buf.iter().fold(0u32, |a, b| a + *b as u32)) + } + + fn resolve(&mut self, _env: Env, output: Self::Output) -> Result { + Ok(output) + } +} + +#[napi] +fn async_reduce_buffer(mut buf: Buffer, env: Env) -> Result> { + Ok(AsyncTask::new(AsyncBuffer { + buf: buf.clone(&env)?, + })) +} diff --git a/memory-testing/index.mjs b/memory-testing/index.mjs index 8d5957dd..65309afe 100644 --- a/memory-testing/index.mjs +++ b/memory-testing/index.mjs @@ -2,5 +2,3 @@ import { createSuite } from './test-util.mjs' await createSuite('tokio-future') await createSuite('serde') - -process.exit(0)