marc2332 / freya

Cross-platform GUI library for 🦀 Rust powered by 🧬 Dioxus and 🎨 Skia.
https://freyaui.dev/
MIT License
1.29k stars 51 forks source link

Signal panicking when being set in a separate thread #603

Open ZeroX-DG opened 2 months ago

ZeroX-DG commented 2 months ago

I ran into this issue when I have a SyncSignal being updated in a thread. I think it got something to do with it being set too quickly between render? 🤔

Here is the error that I got:

thread 'main' panicked at /Users/viethung/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dioxus-signals-0.5.1/src/read.rs:129:38:
called `Result::unwrap()` on an `Err` value: AlreadyBorrowedMut(AlreadyBorrowedMutError { borrowed_mut_at: Location { file: "src/components/content_area.rs", line: 71, col: 49 } })
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at /Users/viethung/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dioxus-signals-0.5.1/src/read.rs:129:38:
called `Result::unwrap()` on an `Err` value: AlreadyBorrowedMut(AlreadyBorrowedMutError { borrowed_mut_at: Location { file: "src/components/content_area.rs", line: 71, col: 49 } })
thread 'main' panicked at /Users/viethung/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.29.15/src/platform_impl/macos/app_state.rs:387:33:
called `Result::unwrap()` on an `Err` value: PoisonError { .. }
thread 'tokio-runtime-worker' panicked at src/components/content_area.rs:71:49:
called `Result::unwrap()` on an `Err` value: Dropped(ValueDroppedError { created_at: Location { file: "src/components/content_area.rs", line: 17, col: 30 } })

Here's a link to that line that the error was complaining about:

https://github.com/ZeroX-DG/raven/blob/112e867d68d61d2b5a78f7cddac9e4f7e02a44a1/src/components/content_area.rs#L71 #

marc2332 commented 2 months ago

Spawning the async task with tokio is probabl causing this, just move the task out of it

spawn(async move {
  while let Ok(event) = terminal_event_rx.recv() {
          match event {
              TerminalEvent::Redraw {
                  lines,
                  cursor,
                  scroll_top,
              } => {
                  *rendered_lines.write() = lines;
                  *rendered_cursor.write() = (cursor.x, cursor.y as usize);
                  *rendered_scroll_top.write() = scroll_top;
              }
          }
      }
  });
ZeroX-DG commented 2 months ago

Moving it outside the tokio task will actually block the main thread & the app freezes. I tried using std::thread::spawn instead of dioxus spawn but the error still persist.

marc2332 commented 2 months ago

Can't you just use a non-blocking, async-friendly channel instead of crossbeam ? Like https://docs.rs/tokio/latest/tokio/sync/mpsc/fn.unbounded_channel.html

ZeroX-DG commented 2 months ago

I've just switched to flume to have async recv channel: https://github.com/ZeroX-DG/raven/blob/db0b86052b4fbc101562990d9678c19df2482dd0/src/components/content_area.rs#L63

But now I run into the old bug in https://github.com/marc2332/freya/issues/602. Maybe something is blocking the thread again 😢 I'll investigate more.

marc2332 commented 2 months ago

Can you try with tokio just in case?

marc2332 commented 2 months ago

Btw I wonder if for the terminal renderer it would be better to use the canvas directly, that would surely be more performant

ZeroX-DG commented 2 months ago

Yep. Same thing happen using tokio channel.

thread 'main' panicked at /Users/viethung/.cargo/git/checkouts/freya-aa88117f1f713ee2/7b68806/crates/core/src/dom/mutations_writer.rs:46:60:
called `Option::unwrap()` on a `None` value

Here's my branch for the tokio channel if you want to check it out: https://github.com/ZeroX-DG/raven/tree/tokio-channel

ZeroX-DG commented 2 months ago

But yeah. Implementing in cavas would indeed be more performant & avoid blocking UI thread problem. I'll try that instead!

marc2332 commented 4 weeks ago

Is this still an issue? 👀

ZeroX-DG commented 4 weeks ago

I have no idea haha. I switched to using canvas & don't run into this issue any more 😂

I'll test the old code with the latest freya version.