tarkah / tickrs

Realtime ticker data in your terminal 📈
MIT License
1.16k stars 58 forks source link

Panic: Too many open files #52

Closed miraclx closed 3 years ago

miraclx commented 3 years ago
Backtrace (most recent call first):
  File "rust:library/core/src/option.rs", line 1268, in core::option::expect_none_failed
  File "<unknown>", line 0, in tickrs_api::client::Client::new
  File "<unknown>", line 0, in tickrs::task::AsyncTask::connect
  File "<unknown>", line 0, in tickrs::event::handle_keys_display_summary
  File "<unknown>", line 0, in tickrs::main
  File "<unknown>", line 0, in std::sys_common::backtrace::__rust_begin_short_backtrace
  File "<unknown>", line 0, in std::rt::lang_start::{{closure}}
  File "rust:library/core/src/ops/function.rs", line 259, in core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once
  File "rust:library/std/src/panicking.rs", line 379, in std::panicking::try::do_call
  File "rust:library/std/src/panicking.rs", line 343, in std::panicking::try
  File "rust:library/std/src/panic.rs", line 396, in std::panic::catch_unwind
  File "rust:library/std/src/rt.rs", line 51, in std::rt::lang_start_internal
  File "<unknown>", line 0, in main
  File "<unknown>", line 0, in __libc_start_main
  File "<unknown>", line 0, in _start

The application panicked (crashed).
  called `Result::unwrap()` on an `Err` value: Io(Os { code: 24, kind: Other, message: "Too many open files" })
in /tickrs-api-0.9.0/src/client.rs, line 193
thread: main
miraclx commented 3 years ago

After some digging into the code, a Result::Err was returned by isahc because of the reinitialization of HTTP clients for every task which isahc itself warns against as it's expensive and inefficient docs.rs [isahc::HttpClient]

A fix for this would be statically creating a crate-wide instantiation of the Client that's shared across all tasks.

tarkah commented 3 years ago

Yeah good call, I'll add that to the api crate. It's interesting because I had this same too many files open error early last year when I first started this project, but it was an upstream bug with the crossterm crate we use. I was originally thinking iti might be from that again.

Thanks for digging into this!

miraclx commented 3 years ago

All good man! I already have a fix locally

I'll setup a PR now

tarkah commented 3 years ago

Awesome! I'll check this out and the other things tomorrow. I assume this was fixed by the PR? How many tabs did you create before getting the panic?

miraclx commented 3 years ago

Great. Not a lot of tabs actually. Just kept switching time frames in the summary view of 4 tickers to see how well and fast it can keep up.