infinyon / node-bindgen

Easy way to write Node.js module using Rust
Apache License 2.0
507 stars 43 forks source link

Supporting ArrayBuffer since electron 21 and higher #309

Closed DmitryAstafyev closed 1 day ago

DmitryAstafyev commented 1 month ago

Issue

Panic on call try_to_js with napi_status == napi_status_napi_no_external_buffers_allowed. Looks like changes comes with electron 21 >.

Probably this is related:

(node:228244) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.

Reproducing

An example is created based on your example of buffer usage. Unpack and do

npm install
npm run build
npm run test

Error trace

thread '<unnamed>' panicked at /tmp/node-bindgen/nj-core/src/error.rs:156:18:
cannot convert: 22
stack backtrace:
   0:     0x75d186336b85 - std::backtrace_rs::backtrace::libunwind::trace::h1a07e5dba0da0cd2
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/../../backtrace/src/backtrace/libunwind.rs:105:5
   1:     0x75d186336b85 - std::backtrace_rs::backtrace::trace_unsynchronized::h61b9b8394328c0bc
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x75d186336b85 - std::sys_common::backtrace::_print_fmt::h1c5e18b460934cff
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/sys_common/backtrace.rs:68:5
   3:     0x75d186336b85 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h1e1a1972118942ad
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x75d186359b2b - core::fmt::rt::Argument::fmt::h07af2b4071d536cd
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/fmt/rt.rs:165:63
   5:     0x75d186359b2b - core::fmt::write::hc090a2ffd6b28c4a
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/fmt/mod.rs:1157:21
   6:     0x75d186334c9f - std::io::Write::write_fmt::h8898bac6ff039a23
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/io/mod.rs:1832:15
   7:     0x75d18633695e - std::sys_common::backtrace::_print::h4e80c5803d4ee35b
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x75d18633695e - std::sys_common::backtrace::print::ha96650907276675e
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x75d186337c19 - std::panicking::default_hook::{{closure}}::h215c2a0a8346e0e0
  10:     0x75d18633795d - std::panicking::default_hook::h207342be97478370
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panicking.rs:298:9
  11:     0x75d1863380b3 - std::panicking::rust_panic_with_hook::hac8bdceee1e4fe2c
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panicking.rs:795:13
  12:     0x75d186337f94 - std::panicking::begin_panic_handler::{{closure}}::h00d785e82757ce3c
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panicking.rs:664:13
  13:     0x75d186337049 - std::sys_common::backtrace::__rust_end_short_backtrace::h1628d957bcd06996
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/sys_common/backtrace.rs:171:18
  14:     0x75d186337cc7 - rust_begin_unwind
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panicking.rs:652:5
  15:     0x75d186085c43 - core::panicking::panic_fmt::hdc63834ffaaefae5
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/panicking.rs:72:14
  16:     0x75d1860986b5 - <nj_core::error::NapiStatus as core::convert::From<u32>>::from::hef8d356215729b30
                               at /tmp/node-bindgen/nj-core/src/error.rs:156:18
  17:     0x75d1860a59bb - <T as core::convert::Into<U>>::into::hcfec45bf5df424ba
                               at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/convert/mod.rs:759:9
  18:     0x75d1860a4258 - <nj_core::buffer::ArrayBuffer as nj_core::convert::TryIntoJs>::try_to_js::h7cfcedbb2cd39465
                               at /tmp/node-bindgen/nj-core/src/buffer.rs:74:48
  19:     0x75d18608c2fc - <core::result::Result<T,E> as nj_core::convert::TryIntoJs>::try_to_js::h151e60d4e733e9c8
                               at /tmp/node-bindgen/nj-core/src/convert.rs:158:24
  20:     0x75d186089a4f - nj_example_buffer::napi_test::{{closure}}::hb23c77b9b2ff1d3f
                               at /tmp/node-bindgen/examples/buffer/src/lib.rs:14:1
  21:     0x75d186086eef - nj_example_buffer::napi_test::h53dc7d15ba96a2de
                               at /tmp/node-bindgen/examples/buffer/src/lib.rs:14:1
  22:     0x61069a82a7ec - <unknown>
fatal runtime error: failed to initiate panic, error 5

buffer.zip

DmitryAstafyev commented 1 month ago

Hello @sehz . Any chance to take a look at this issue? Sending bytes as Vec<u8> works quite slowly as soon as there are iterating and cloning. I guess using of Buffer should give better performance, but it doesn't work with electron )

DmitryAstafyev commented 1 month ago

@morenol @sehz hello guys, any chance to take a look on it ? :/

sehz commented 1 month ago

Can you try upgrading https://github.com/infinyon/node-bindgen/tree/master/nj-sys? Will try take look at later part of the week

DmitryAstafyev commented 1 month ago

Can you try upgrading https://github.com/infinyon/node-bindgen/tree/master/nj-sys? Will try take look at later part of the week

Hello @sehz I've did make generate (in context nj-sys) and after once again tried to build example (which I've provided here) and run it. Unfortunately, the same result.

sehz commented 1 month ago

It's in here: https://github.com/infinyon/node-bindgen/blob/master/nj-core/src/buffer.rs. Nothing obvious yet.

DmitryAstafyev commented 1 month ago

@sehz @morenol hello guys. Any news?

sehz commented 4 weeks ago

Been busy this week. Will try to find sometime over weekend to see what's going on

DmitryAstafyev commented 3 weeks ago

@sehz sorry to ping you again. Any news?

sehz commented 3 weeks ago

I am trying to reproduce it. What version of node?

DmitryAstafyev commented 1 week ago

@sehz sorry for delay with the reply, I've missed your message. The issue comes not with node, but with electron. I've attached example to this issue. Actually it's original example from repo, but modified to be runned in the context of electron. As for running in the context of node - there are no issues at all.

DmitryAstafyev commented 2 days ago

@sehz @morenol Hello guys. Actually, I think here is nothing to do with ArrayBuffer implementation. It's a restriction of electron. As a workaround, I'm suggesting adding implementation for SafeBuffer, which can be used without issue with electron to send [u8] bytes to JS context as well with minimal performance losses.

Please take a look #318

DmitryAstafyev commented 1 day ago

Thanks @sehz . Could you please also update it on crates.io )

sehz commented 1 day ago

@morenol can you release?

morenol commented 20 hours ago

@DmitryAstafyev crates released. Is it all working for you as expected?