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

feat: allow configuration to be read from stdin #5

Open
wants to merge 3 commits into
base: ubuntu/v0.32.1
Choose a base branch
from

Conversation

kgilmer
Copy link
Member

@kgilmer kgilmer commented Feb 10, 2024

NOTE: not intended for merging as-is. Causing crash upon start in async machinery, unsure of why or how to fix. Looking for suggestions

CC @SoumyaRanjanPatnaik @cfsmp3

@kgilmer
Copy link
Member Author

kgilmer commented Feb 10, 2024

Crash deets. Also tried the stock config provided on the same version branch of i3status-rs, same crash.

$ cat /etc/regolith/i3status-rust/config.toml | target/debug/i3status-rs -
{"version": 1, "click_events": true}
[
2
[],
2
[],
thread 'main' panicked at 'Unfold must not be polled after it returned `Poll::Ready(None)`', /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/unfold.rs:108:21
stack backtrace:
   0: std::panicking::begin_panic
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/panicking.rs:610:12
   1: <futures_util::stream::unfold::Unfold<T,F,Fut> as futures_core::stream::Stream>::poll_next
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/unfold.rs:108:21
   2: <core::pin::Pin<P> as futures_core::stream::Stream>::poll_next
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:120:9
   3: futures_util::stream::stream::StreamExt::poll_next_unpin
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/stream/mod.rs:1632:9
   4: <futures_util::stream::stream::next::Next<St> as core::future::future::Future>::poll
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/stream/next.rs:32:9
   5: i3status_rs::BarState::process_event::{{closure}}::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/macros/select.rs:524:49
   6: <tokio::future::poll_fn::PollFn<F> as core::future::future::Future>::poll
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/future/poll_fn.rs:58:9
   7: i3status_rs::BarState::process_event::{{closure}}
             at ./src/lib.rs:353:9
   8: i3status_rs::BarState::run_event_loop::{{closure}}
             at ./src/lib.rs:448:60
   9: i3status_rs::main::{{closure}}
             at ./src/main.rs:55:40
  10: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/future/future.rs:125:9
  11: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:665:57
  12: tokio::runtime::coop::with_budget
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/coop.rs:107:5
  13: tokio::runtime::coop::budget
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/coop.rs:73:5
  14: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:665:25
  15: tokio::runtime::scheduler::current_thread::Context::enter
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:410:19
  16: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:664:36
  17: tokio::runtime::scheduler::current_thread::CoreGuard::enter::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:743:68
  18: tokio::runtime::context::scoped::Scoped<T>::set
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context/scoped.rs:40:9
  19: tokio::runtime::context::set_scheduler::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context.rs:176:26
  20: std::thread::local::LocalKey<T>::try_with
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/thread/local.rs:252:16
  21: std::thread::local::LocalKey<T>::with
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/thread/local.rs:228:9
  22: tokio::runtime::context::set_scheduler
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context.rs:176:9
  23: tokio::runtime::scheduler::current_thread::CoreGuard::enter
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:743:27
  24: tokio::runtime::scheduler::current_thread::CoreGuard::block_on
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:652:19
  25: tokio::runtime::scheduler::current_thread::CurrentThread::block_on::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:175:28
  26: tokio::runtime::context::runtime::enter_runtime
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context/runtime.rs:65:16
  27: tokio::runtime::scheduler::current_thread::CurrentThread::block_on
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:167:9
  28: tokio::runtime::runtime::Runtime::block_on
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/runtime.rs:347:47
  29: i3status_rs::main
             at ./src/main.rs:23:18
  30: core::ops::function::FnOnce::call_once
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace

Copy link
Contributor

@cfsmp3 cfsmp3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments but I can't actually test the diff from where I am. Hopefully the stdin comment will help.

src/main.rs Outdated

if size == 0 {
return Err(i3status_rs::errors::Error { kind: ErrorKind::Config, message: None, cause: None, block: None });
let size = std::io::stdin()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably need to use the tokio version of stdin. https://docs.rs/tokio/latest/tokio/io/fn.stdin.html

@@ -104,10 +104,10 @@ where
{
let source = match file_path {
Some(msg) => msg.display().to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible a if let else construct is more readable than a match to check an Option. Something like

if let Some(path) = file_path { path} else { "(from stdin)"} 

@kgilmer
Copy link
Member Author

kgilmer commented Feb 11, 2024

Thanks for the ideas Carlos. Yeah I'd tried that as well (pushed the change), and here's what the crash bt looks like:

$ cat /etc/regolith/i3status-rust/config.toml | ./target/debug/i3status-rs -
{"version": 1, "click_events": true}
[
[],
[],
[],
[],
[{"full_text":"   1TB ","color":"#93A1A1FF","name":"0","instance":"1:","markup":"pango"}],
[{"full_text":"   1TB ","color":"#93A1A1FF","name":"0","instance":"1:","markup":"pango"}],
[{"full_text":"   1TB ","color":"#93A1A1FF","name":"0","instance":"1:","markup":"pango"}],
thread 'main' panicked at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/unfold.rs:108:21:
Unfold must not be polled after it returned `Poll::Ready(None)`
stack backtrace:
   0: std::panicking::begin_panic
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/panicking.rs:686:12
   1: <futures_util::stream::unfold::Unfold<T,F,Fut> as futures_core::stream::Stream>::poll_next
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/unfold.rs:108:21
   2: <core::pin::Pin<P> as futures_core::stream::Stream>::poll_next
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:120:9
   3: futures_util::stream::stream::StreamExt::poll_next_unpin
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/stream/mod.rs:1632:9
   4: <futures_util::stream::stream::next::Next<St> as core::future::future::Future>::poll
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/stream/next.rs:32:9
   5: i3status_rs::BarState::process_event::{{closure}}::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/macros/select.rs:524:49
   6: <tokio::future::poll_fn::PollFn<F> as core::future::future::Future>::poll
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/future/poll_fn.rs:58:9
   7: i3status_rs::BarState::process_event::{{closure}}
             at ./src/lib.rs:353:9
   8: i3status_rs::BarState::run_event_loop::{{closure}}
             at ./src/lib.rs:439:61
   9: i3status_rs::main::{{closure}}
             at ./src/main.rs:69:41
  10: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/future/future.rs:125:9
  11: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:665:57
  12: tokio::runtime::coop::with_budget
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/coop.rs:107:5
  13: tokio::runtime::coop::budget
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/coop.rs:73:5
  14: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:665:25
  15: tokio::runtime::scheduler::current_thread::Context::enter
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:410:19
  16: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:664:36
  17: tokio::runtime::scheduler::current_thread::CoreGuard::enter::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:743:68
  18: tokio::runtime::context::scoped::Scoped<T>::set
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context/scoped.rs:40:9
  19: tokio::runtime::context::set_scheduler::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context.rs:176:26
  20: std::thread::local::LocalKey<T>::try_with
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/thread/local.rs:270:16
  21: std::thread::local::LocalKey<T>::with
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/thread/local.rs:246:9
  22: tokio::runtime::context::set_scheduler
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context.rs:176:9
  23: tokio::runtime::scheduler::current_thread::CoreGuard::enter
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:743:27
  24: tokio::runtime::scheduler::current_thread::CoreGuard::block_on
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:652:19
  25: tokio::runtime::scheduler::current_thread::CurrentThread::block_on::{{closure}}
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:175:28
  26: tokio::runtime::context::runtime::enter_runtime
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context/runtime.rs:65:16
  27: tokio::runtime::scheduler::current_thread::CurrentThread::block_on
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:167:9
  28: tokio::runtime::runtime::Runtime::block_on
             at /home/kgilmer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/runtime.rs:347:47
  29: i3status_rs::main
             at ./src/main.rs:24:18
  30: core::ops::function::FnOnce::call_once
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@SoumyaRanjanPatnaik
Copy link
Contributor

I managed to fix this issue. Check this commit on stdin-fix-stream-next branch.

@SoumyaRanjanPatnaik
Copy link
Contributor

Turns out for some unknown reason (that I haven't yet figured out), the crash only occurred when the config read from stdin (or from any other source) caused i3status-rs to fail. To reproduce, try the following configs:

  1. This should work
    [theme]
    theme = "semi-native"
    
    [icons]
    icons = "awesome4"
    
    
  2. This should panic.
    [theme]
    theme = "semi-native"
    
    [icons]
    icons = "awesome4"
    
    [[block]]
    block = "net"
    

To check if this was an issue with stdin, I tried passing a constant string instead of reading from stdin. Saw the same behaviour:

  1. This works
    diff --git a/src/main.rs b/src/main.rs
    index 22adb842..499f74f5 100644
    --- a/src/main.rs
    +++ b/src/main.rs
    @@ -30,14 +30,23 @@ fn main() {
                 let mut config: Config = match args.config.as_str() {
                     STDIN_FILE_DESIGNATOR => {
                         // read from stdin
    -                    let mut config_str = String::new();
    +                    let config_str = String::from(r#"
    +[theme]
    +theme = "semi-native"
    +
    +[icons]
    +icons = "awesome4"
    +                    "#);
     
                         if size == 0 {
                             return Err(i3status_rs::errors::Error {
  2. This doesn't
diff --git a/src/main.rs b/src/main.rs
index 22adb842..0b6b1d9f 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -30,23 +30,25 @@ fn main() {
             let mut config: Config = match args.config.as_str() {
                 STDIN_FILE_DESIGNATOR => {
                     // read from stdin
-                    let mut config_str = String::new();
+                    let config_str = String::from(r#"
+[theme]
+theme = "semi-native"
+
+[icons]
+icons = "awesome4"
+
+[[block]]
+block = "net"
+                    "#);
 
                     util::deserialize_toml(&config_str, None)?
                 }

@SoumyaRanjanPatnaik
Copy link
Contributor

Unfold must not be polled after it returned Poll::Ready(None)

This is the message that we get in the backtrace. Looked it up. This happens when next is called on a Stream that is already finished (marked by next returning None). The easiest way to fix it is by creating a FusedStream (again looked it up) which tracks if the stream has already finished and returns None for subsequent calls to next.

The event loop polls deals with several instances of Stream. stdin-fix-stream-next fixes the issue by converting them into FusedStream.

@SoumyaRanjanPatnaik
Copy link
Contributor

Still unsure about why this doesn't happen when the config is read from a file. Quite curious actually.

Frankly, I have no idea about what I'm doing 😅. I'm not very familiar with most of the async rust usage in the codebase.

@kgilmer
Copy link
Member Author

kgilmer commented Feb 13, 2024

Frankly, I have no idea about what I'm doing 😅.

..Oh I wouldn't say that haha. Thanks for diving into this weird problem Soumya. This is my first Rust experience where the type system and borrow checker didn't provide a great debugging experience. I guess I'm somewhat relieved that wasn't something super obvious, as I spent a lot of time trying to rootcause this myself before giving up.

...This happens when next is called on a Stream that is already finished (marked by next returning None).

I had gotten roughly here in my debugging but was really uncertain about the next steps, as reasoning about how this had been working in the past in addition to general lack of familiarity w/ async rust.

I wonder if these fixes make sense to push upstream.. FWIW this change is to allow for config partials in i3status-rs, to enable the i3xrocks-style "configure via package manager" approach.

@kgilmer
Copy link
Member Author

kgilmer commented Feb 13, 2024

@SoumyaRanjanPatnaik in your diff that fixes the crash, you're pinning the stream. Any insights you can provide as to why that was necessary?

@SoumyaRanjanPatnaik
Copy link
Contributor

This is my first Rust experience where the type system and borrow checker didn't provide a great debugging experience.

Same for me as well. It was really hard to figure out where the error originated. The backtrace wasn't helpful as well.

I had gotten roughly here in my debugging but was really uncertain about the next steps, as reasoning about how this had been working in the past

To be frank, I still haven't figured out why it had been working in the past. In fact, even passing a &'static str instead of reading from input caused the program to crash (only if the config defined any blocks).

I wonder if these fixes make sense to push upstream..

Might be a good idea to check with them.

FWIW this change is to allow for config partials in i3status-rs, to enable the i3xrocks-style "configure via package manager" approach.

I figured that much. Tbh, I do like the idea of being able to read config from stdin. However I wonder what the behaviour of i3status-rs will be after this patch when it receives a SIGUSR2 signal (which is supposed to trigger a restart). You might wanna check out these snippets.

signal_hook::iterator::Signals::new([signal_hook::consts::SIGUSR2])
.unwrap()
.forever()
.next()
.unwrap();
restart();

fn restart() -> ! {
use std::env;
use std::ffi::CString;
use std::os::unix::ffi::OsStringExt;
// On linux this line should be OK
let exe = CString::new(env::current_exe().unwrap().into_os_string().into_vec()).unwrap();
// Get current arguments
let mut arg: Vec<CString> = env::args_os()
.map(|a| CString::new(a.into_vec()).unwrap())
.collect();
// Add "--no-init" argument if not already added
let no_init_arg = CString::new("--no-init").unwrap();
if !arg.iter().any(|a| *a == no_init_arg) {
arg.push(no_init_arg);
}
// Restart
nix::unistd::execvp(&exe, &arg).unwrap();
unreachable!();
}

To summarize, i3status-rs restarts with the same args, which means it would expect to receive input via stdin. Any ideas to deal with this?

@SoumyaRanjanPatnaik
Copy link
Contributor

@SoumyaRanjanPatnaik in your diff that fixes the crash, you're pinning the stream. Any insights you can provide as to why that was necessary?

Calling fuse() was the important bit. It returns a new FusedStream object, which keeps track of whether the underlying stream has finished or not, and returns None if it has.

Pinning was just necessary to satisfy the BoxedStream type.

type BoxedStream<T> = Pin<Box<dyn FusedStream<Item = T>>>;

The streams were pinned prior to my changes as well using either boxed or boxed_local. So I went ahead and pinned the FusedStream returned by fuse() to satisfy the typing constraints.

@kgilmer kgilmer mentioned this pull request May 3, 2024
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

Successfully merging this pull request may close these issues.

3 participants