Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supporting ArrayBuffer since electron 21 and higher #309

Closed
DmitryAstafyev opened this issue Jul 24, 2024 · 14 comments
Closed

Supporting ArrayBuffer since electron 21 and higher #309

DmitryAstafyev opened this issue Jul 24, 2024 · 14 comments

Comments

@DmitryAstafyev
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@sehz
Copy link
Collaborator

sehz commented Jul 30, 2024

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
Copy link
Contributor Author

DmitryAstafyev commented Jul 31, 2024

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
Copy link
Collaborator

sehz commented Aug 1, 2024

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

@DmitryAstafyev
Copy link
Contributor Author

@sehz @morenol hello guys. Any news?

@sehz
Copy link
Collaborator

sehz commented Aug 8, 2024

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

@DmitryAstafyev
Copy link
Contributor Author

@sehz sorry to ping you again. Any news?

@sehz
Copy link
Collaborator

sehz commented Aug 17, 2024

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

@DmitryAstafyev
Copy link
Contributor Author

@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
Copy link
Contributor Author

DmitryAstafyev commented Sep 5, 2024

@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
Copy link
Contributor Author

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

@sehz
Copy link
Collaborator

sehz commented Sep 5, 2024

@morenol can you release?

@morenol
Copy link
Contributor

morenol commented Sep 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants