panic when accessing State::write in use_future

[milesj]

Looks like the recent State::write changes will panic when used within a use_future and the component is unmounted.

Error:
  × Main thread panicked.
  ├─▶ at /Users/miles/.cargo/registry/src/index.crates.io-6f17d22bba15001f/iocraft-0.5.1/src/hooks/use_state.rs:199:31
  ╰─▶ called `Result::unwrap()` on an `Err` value: Dropped(ValueDroppedError { created_at: Location { file: "/Users/miles/.cargo/registry/src/index.crates.io-6f17d22bba15001f/iocraft-
      0.5.1/src/hooks/use_state.rs", line: 72, col: 32 } })

A try_write is probably enough to solve this problem.

[ccbrown]

A try_write function would be trivial to add, but maybe there's a way to eliminate this pitfall altogether. Can you provide more details on how you're running into this?

I tried reproducing with the following, but wasn't able to. Are you referencing the state from the parent's hook or something?

use iocraft::prelude::*;
use std::time::Duration;

#[component]
fn Counter(mut hooks: Hooks) -> impl Into<AnyElement<'static>> {
    let mut count = hooks.use_state(|| 0);

    hooks.use_future(async move {
        loop {
            smol::Timer::after(Duration::from_millis(100)).await;
            count += 1;
        }
    });

    element! {
        Text(color: Color::Blue, content: format!("counter: {}", count))
    }
}

#[component]
fn Container(mut hooks: Hooks) -> impl Into<AnyElement<'static>> {
    let mut should_show = hooks.use_state(|| true);

    hooks.use_terminal_events({
        move |event| match event {
            TerminalEvent::Key(KeyEvent { code, kind, .. }) if kind != KeyEventKind::Release => {
                match code {
                    KeyCode::Char('x') => should_show.set(!should_show.get()),
                    _ => {}
                }
            }
            _ => {}
        }
    });

    element! {
        Box {
            #(if should_show.get() { Some(element!(Counter)) } else { None })
        }
    }
}

fn main() {
    smol::block_on(element!(Container).render_loop()).unwrap();
}

[ccbrown]

I've put together a PR with try_read and try_write methods, here: #49

But before merging, I'd like to understand better what kind of use-case you have that would trigger that panic.

[milesj]

@ccbrown It's this line: https://github.com/moonrepo/starbase/blob/progress-state/crates/console/src/components/progress.rs#L135

Basically, I have a progress bar component and this Estimator instance that is used to track information like duration, ETA, bytes per second, etc. This needs to continually update so it's in a loop.

I have to spawn a nested task, since the outer use_future already has a loop, and you can't call use_future multiple times.

[ccbrown]

Ah, yeah, that would probably do it. Because you're launching the task with tokio::spawn, it continues to live on, after the component is unmounted. Note that handle.abort() is not guaranteed to be called, because the async block around it is simply dropped when the component unmounts.

I think you'd want to use use_future (twice) instead of tokio::spawn, like so:

    hooks.use_future(async move {
        loop {
            sleep(Duration::from_millis(150)).await;
            estimator.write().record(value.get(), Instant::now());
        }
    });

    hooks.use_future(async move {
        let mut receiver = reporter.subscribe();

        while let Ok(state) = receiver.recv().await {
            match state {
                ProgressState::Wait(val) => {
                    sleep(val).await;
                }
                ProgressState::Exit => {
                    should_exit.set(true);
                    break;
                }
                ProgressState::Max(val) => {
                    max.set(val);
                }
                ProgressState::Message(val) => {
                    message.set(val);
                }
                ProgressState::Prefix(val) => {
                    prefix.set(val);
                }
                ProgressState::Suffix(val) => {
                    suffix.set(val);
                }
                ProgressState::Value(val) => {
                    if val >= max.get() {
                        value.set(max.get());
                        should_exit.set(true);
                        break;
                    } else {
                        value.set(val);
                    }
                }
                _ => {}
            };
        }
    });

That way you're guaranteed to have all your async tasks cleaned up when the component goes away.

[milesj]

I've tried that but only 1 of the futures would actually run. Let me try again, maybe my component was in a bad state.

[milesj]

@ccbrown Yeah, changed it to this, but the 2nd future never runs. moonrepo/starbase@39c9222

Here's the output in the console:

────────────────────
ProgressBar REPORTER
╾───────────────────
ProgressLoader REPORTER

If I swap the use_future order, it becomes:

────────────────────
ProgressBar LOOP
╾───────────────────
ProgressLoader LOOP

[ccbrown]

@milesj can you try the same thing, but do the print using the use_output hook? If you use println directly, the output of the second one might just be getting erased during rendering.

This will make sure the output is always printed above the TUI (the change in indentation makes this diff look much bigger than it is):

diff --git a/crates/console/src/components/progress.rs b/crates/console/src/components/progress.rs
index df2dccb..7a3b310 100644
--- a/crates/console/src/components/progress.rs
+++ b/crates/console/src/components/progress.rs
@@ -126,52 +126,57 @@ pub fn ProgressBar<'a>(
 
     let reporter = props.reporter.take();
 
-    hooks.use_future(async move {
-        println!("ProgressBar REPORTER");
+    let (stdout, _) = hooks.use_output();
 
-        let Some(reporter) = reporter else {
-            return;
-        };
+    hooks.use_future({
+        let stdout = stdout.clone();
+        async move {
+            stdout.println("ProgressBar REPORTER");
 
-        let mut receiver = reporter.subscribe();
+            let Some(reporter) = reporter else {
+                return;
+            };
 
-        while let Ok(state) = receiver.recv().await {
-            match state {
-                ProgressState::Wait(val) => {
-                    sleep(val).await;
-                }
-                ProgressState::Exit => {
-                    should_exit.set(true);
-                    break;
-                }
-                ProgressState::Max(val) => {
-                    max.set(val);
-                }
-                ProgressState::Message(val) => {
-                    message.set(val);
-                }
-                ProgressState::Prefix(val) => {
-                    prefix.set(val);
-                }
-                ProgressState::Suffix(val) => {
-                    suffix.set(val);
-                }
-                ProgressState::Value(val) => {
-                    if val >= max.get() {
-                        value.set(max.get());
+            let mut receiver = reporter.subscribe();
+
+            while let Ok(state) = receiver.recv().await {
+                match state {
+                    ProgressState::Wait(val) => {
+                        sleep(val).await;
+                    }
+                    ProgressState::Exit => {
                         should_exit.set(true);
                         break;
-                    } else {
-                        value.set(val);
                     }
-                }
-                _ => {}
-            };
+                    ProgressState::Max(val) => {
+                        max.set(val);
+                    }
+                    ProgressState::Message(val) => {
+                        message.set(val);
+                    }
+                    ProgressState::Prefix(val) => {
+                        prefix.set(val);
+                    }
+                    ProgressState::Suffix(val) => {
+                        suffix.set(val);
+                    }
+                    ProgressState::Value(val) => {
+                        if val >= max.get() {
+                            value.set(max.get());
+                            should_exit.set(true);
+                            break;
+                        } else {
+                            value.set(val);
+                        }
+                    }
+                    _ => {}
+                };
+            }
         }
     });
 
     hooks.use_future(async move {
-        println!("ProgressBar LOOP");
+        stdout.println("ProgressBar LOOP");
 
         loop {
             sleep(Duration::from_millis(150)).await;

And that appears to be working to me:

[milesj]

Ah you're right. This has been throwing me off thinking my implementation was wrong. Let me try and tidy everything back up and see if I run into any other issues.

[milesj]

@ccbrown Ok I believe I got it all working. Had to rewrite my Progress component, but it's working much better now. I do have 1 question, unrelated to the original issue.

For the progress component, I need a way to update state from outside the component, because I need to inject the current progress numbers. I'm currently achieving this using tokio's broadcast channel (I need fan out style). You can see this here:

• The impl: https://github.com/moonrepo/starbase/pull/118/files#diff-b046ffc3492bd515cbfbe28b2660bfa664fc027f2425fe51352123147323fdacR34

• Which is passed as a reporter prop: https://github.com/moonrepo/starbase/pull/118/files#diff-b046ffc3492bd515cbfbe28b2660bfa664fc027f2425fe51352123147323fdacR172

• And then handled in the use_future loop (which you've seen already): https://github.com/moonrepo/starbase/pull/118/files#diff-b046ffc3492bd515cbfbe28b2660bfa664fc027f2425fe51352123147323fdacR195

This works perfectly when rendering a single progress bar in the terminal. However, when I want to render multiple progress bars, it seems like only the 1st bar in the stack actually updates, and the other ones do not. Here's a screenshot of what happens:

When I enabled logging, the other tools are actually being processed in the background, but the UI never reflects this. At first I thought this may be an issue where iocraft was creating a new sender/receiver on each render, so I created this enum to support owned and shared data: https://github.com/moonrepo/starbase/pull/118/files#diff-9980d92615c1ff8bf526ee646ea6b6652d37af620bc7fd8ffeb870c334a28bbbR67

This didn't solve the problem. I'm debugged this quite a bit but not really sure what's going on. So my question is, have you thought about this problem at all? Basically how to update state from outside the component?

For context and reference, here's the PR that's implementing the progress:

• The component that renders all progress bars. The data is a hashmap of ID -> props for the inner component: https://github.com/moonrepo/proto/pull/683/files#diff-5241d26bb66b269b29573cf53c6e2f8c001ed99ae9019401d23cad9fcb7a50bdR10

• The component that renders a single bar (which is rendered by the component above): https://github.com/moonrepo/proto/pull/683/files#diff-be1ef029b20e28108817398ecb3e2587f196e20bcaa7a892e7a985650d8e875aR7

• Loop where I create the tools hashmap: https://github.com/moonrepo/proto/pull/683/files#diff-d3d8c1ff8df8f96e8b1ec67fb686f97ffc9af36743a401c883cbf08e5affc4daR250

• Where the render actually happens: https://github.com/moonrepo/proto/pull/683/files#diff-d3d8c1ff8df8f96e8b1ec67fb686f97ffc9af36743a401c883cbf08e5affc4daR320

[ccbrown]

Ah, this looks like an iocraft bug that occurs when "key" isn't specified for items added via an iterator. To make sure state is maintained optimally, elements rendered from an iterator should have a key, similar to how it is with React.

I think if you do this, it'll solve your issue:

diff --git a/crates/cli/src/components/install_all_progress.rs b/crates/cli/src/components/install_all_progress.rs
index 1cb2866d..f31ad12e 100644
--- a/crates/cli/src/components/install_all_progress.rs
+++ b/crates/cli/src/components/install_all_progress.rs
@@ -48,7 +48,7 @@ pub fn InstallAllProgress<'a>(
         Stack {
             #(props.tools.iter().map(|(id, inner_props)| {
                 element! {
-                    Box {
+                    Box(key: id.clone()) {
                         Box(
                             justify_content: JustifyContent::End,
                             width: props.id_width as u16,

But iocraft should be able to behave better when it's not provided, or at least warn the user. I'll figure out a way to address this.

[milesj]

Yesss, that worked. Thank you 🙏 It renders beautifully now.

Thanks for this lib. It has made all this terminal stuff super easy to implement!

[ccbrown]

Glad to hear it! I just published 0.5.2, which has some fixes/improvements brought up by this thread. Using key is still recommended, but iocraft will be a lot smarter now if it's omitted, so it should no longer be possible to get in a situation where hooks just mysteriously don't fire. And in case it's useful, that release also includes try_ methods for state.

[milesj]

Awesome work, thanks again.