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

thread 'main' panicked on TextDecoder().decode() #6649

Closed
marcosc90 opened this issue Jul 5, 2020 · 3 comments · Fixed by #7395
Closed

thread 'main' panicked on TextDecoder().decode() #6649

marcosc90 opened this issue Jul 5, 2020 · 3 comments · Fixed by #7395
Labels
bug Something isn't working correctly deno_core Changes in "deno_core" crate are needed

Comments

@marcosc90
Copy link
Contributor

marcosc90 commented Jul 5, 2020

When decoding a big buffer Deno crashes.

new TextDecoder().decode(new Uint8Array(2 ** 29))
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', core/bindings.rs:677:5
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1069
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1504
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:218
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:511
  11: rust_begin_unwind
             at src/libstd/panicking.rs:419
  12: core::panicking::panic_fmt
             at src/libcore/panicking.rs:111
  13: core::panicking::panic
             at src/libcore/panicking.rs:54
  14: deno_core::bindings::decode
  15: <extern "C" fn(A0) .> R as rusty_v8::support::CFnFrom<F>>::mapping::c_fn
  16: _ZN2v88internal25FunctionCallbackArguments4CallENS0_15CallHandlerInfoE
             at ../../../../v8/src/api/api-arguments-inl.h:158
  17: _ZN2v88internal12_GLOBAL__N_119HandleApiCallHelperILb0EEENS0_11MaybeHandleINS0_6ObjectEEEPNS0_7IsolateENS0_6HandleINS0_10HeapObjectEEESA_NS8_INS0_20FunctionTemplateInfoEEENS8_IS4_EENS0_16BuiltinArgumentsE
             at ../../../../v8/src/builtins/builtins-api.cc:111
  18: _ZN2v88internalL26Builtin_Impl_HandleApiCallENS0_16BuiltinArgumentsEPNS0_7IsolateE
             at ../../../../v8/src/builtins/builtins-api.cc:141
  19: Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
fatal runtime error: failed to initiate panic, error 5
Aborted (core dumped

The snippet returns "" on Chrome


Deno version: 1.1.3

@kitsonk
Copy link
Contributor

kitsonk commented Jul 6, 2020

Was there a real situation where you ran into this?

For various reasons, there is a limit to the buffer between Rust and the isolate. In this case, I suspect it was exceeded. We could check in advance the byte length before sending it in the op, but it would be difficult to split it across multiple ops, would need a lot of logic to deal with multi byte encoded chars to make sure you didn't cut off a char.

@marcosc90
Copy link
Contributor Author

Was there a real situation where you ran into this?

A friend was trying to parse a large CSV (~600MB) using Deno.readTextFile('./large.csv') and got the error. We can all agree that a Reader should be used in this case, but it's not uncommon to see scripts that load everything into memory.

We could check in advance the byte length before sending it in the op, but it would be difficult to split it across multiple ops

Throwing a better error until a more robust solution is in place I think it's a step in the right direction.

@bartlomieju bartlomieju added bug Something isn't working correctly deno_core Changes in "deno_core" crate are needed labels Jul 14, 2020
@bartlomieju
Copy link
Member

Throwing a better error until a more robust solution is in place I think it's a step in the right direction.

Agreed, there shouldn't be unwrap() but actual check if allocation succeeded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly deno_core Changes in "deno_core" crate are needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants