tarkah/tickrs

Crash with --time-frame 1D outside of trading hours

wullewutz opened this issue · 15 comments

tickrs hangs when time-frame is set to 1D when started before a certain time (maybe before trading start on NYSE?).
To mitigate this, i can start with -t 1W, but as soon as i change to 1D inside the application, it also hangs.

Can reproduce this on Linux Arch with alacritty terminal as well as WSL2 (Ubuntu) on Windows Terminal.
My timezone is GMT+1 (Berlin).
At the time of filing this issue (8:20 GMT) the crash occurs. I observed the same yesterday morning.
Later in the day, everything works as expected.

Ahh yeah, this is probably happening sometime between midnight and 4am EST before the premarket opens. I'm sure the API switches over to the new day, but doesn't have data yet.

I don't think this used to happen before I added pre/post market data. It'd be helpful to know what the API request is returning so I can handle the error properly.

I'll post a few API links if you could test them for me at the same time tomorrow? I'll try to test tonight after 9pm PST to see if I can recreate.

Is the program crashing and if so, do you have the backtrace that gets printed?

No sorry, the program did not really crash but hangs and remains inresponsive.
Have to kill it or better close the terminal since it messes up the cursor as well.
I also noticed an increasing memory amount in the Windows task manager for the tickrs process. Could be a memory leak as well maybe?

I also cloned your repo run it with "cargo run" (Arch linux) to see if it behaves differently when build in debug mode, but it didn't change anything.

Feel free to post some API links. I'm glad to help and test tomorrow.

Awesome App by the way! I really like it so far.

What version are you running? I released 0.10.0 yesterday with some major optimizations. You can get the latest version here: https://github.com/tarkah/tickrs/releases/tag/v0.10.1.

Assuming you're on >= 0.10.0 already, I'll need to figure out what's causing it to hang. What does your memory usage look like on startup vs when it halts? I'll try to throw it at Valgrind today to see if I can find any leaks. Though it's pretty difficult to have memory issues w/ Rust, it's not impossible. And I'm using some smart pointers in places that could in theory make it happen.

Note that memory usage should increase as you switch between time frames and as the day goes on. I cache the data from each time frame after its received for the first time so that switching back is more seamless. And obviously as the day goes on, more data is received from the API. But this should be a few MB at most I'd think.

With 4 tickers showing in summary mode, with all time frames cached, I'm at ~32MB. I'll try to keep this instance open all day and overnight to see if it balloons and becomes unresponsive.

Appreciate the issue so I can further improve this. Glad you are enjoying it!

Ahh, I see you're testing on Windows. Let me run a version there as well to see if there's any difference.

Edit: NVM, you're running under WSL2. I'm doing the same now for my test

On windows / powershell, running w/ 4 stocks in summary mode, all time frames cached, I'm sitting at 7.4MB. Wow, memory usage is way lower on windows... interesting. Anyway, I'll let this one run all night as well and see if things change.

Ok, I think I know what is deadlocking the program... I'll push out a PR w/ fix here shortly. It'd be great if you can test tonight on both versions to validate it fixes things.

Ok, can you try running from source from #59 tomorrow? I'm 99.9% sure this was the problem as it appears I can create a "never ending" iterator when the API updates for the new trading day, which causes a deadlock since it's never ending. I've added some logic to prevent this from happening.

And to follow up, there doesn't appear to be any memory leaks. I've been running for a few hours now and memory has only bumped up a single time by a few hundred KB, most likely due to a new Vector allocation to handle more pricing data.

Awesome! I'll pull tomorrow morning and report here.

Great, thanks!

I just tested old vs the PR and looks like it fixes it! Hopefully you see the same and I can get this merged tomorrow.

Here my testlog:
Start time: 9:15 GMT
Environment WSL2 (Ubuntu), rustc 1.49.0 (stable), Windows Terminal 1.5.10271.0

Pre-test with tickrs 0.10.0 (current master):
open Terminal
cargo run -- -s GME -t 1D
(Observation: tickrs hangs, CPU > 13%, Memory usage rising fast to >100MB in <1min)
close Terminal

Test with deadlock fix:
open Terminal
git checkout origin/fix/infinite-iterator-deadlock
cargo clean
cargo run -- -s GME -t 1D
(Observation: tickrs starts and shows gui as expected. CPU@ <1%, Memory usage @12,5MB fix)

Conclusion:
The issue seems to be fixed. Ready to merge. Well done!

Awesome! Thanks for confirming