wishawa / async_ui

Lifetime-Friendly, Component-Based, Retained-Mode UI Powered by Async Rust
Mozilla Public License 2.0
548 stars 11 forks source link

thread main panicked at invalid unbind in list.rs #5

Open omac777 opened 1 year ago

omac777 commented 1 year ago

Firstly, I want to say awesome work. Your approach is straighforward and your code in the main.rs for both the native gtk-counter and the gtk-hackernews is easy to follow. Not an easy feat.

On to the bug report. gtk-hackernews at first glance works as expected. You click the "Load More Stories" button and you can see the vertical scroll bar size shrinking indicating the number of list items grew. You click it 7 times and yes the list item count grew. Then if you scroll and click select the last item of the list, then click the "Load More Stories" button, it panics with the following error:

 $ RUST_BACKTRACE=1 ../../target/release/gtk-hackernews
thread 'main' panicked at 'invalid unbind', /home/davidm/async_ui/async_ui_gtk/src/components/list.rs:240:57
stack backtrace:
   0: rust_begin_unwind
             at /rustc/edf0182213a9e30982eb34f3925ddc4cf5ed3471/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/edf0182213a9e30982eb34f3925ddc4cf5ed3471/library/core/src/panicking.rs:65:14
   2: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   3: <async_ui_gtk::components::ElementFuture<F> as core::future::future::Future>::poll
   4: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   5: scoped_tls::ScopedKey<T>::set
   6: <scoped_async_spawn::common::Inner<F> as core::future::future::Future>::poll
   7: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   8: async_task::raw::RawTask<F,T,S>::run
   9: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  10: glib::main_context::<impl glib::auto::main_context::MainContext>::with_thread_default
  11: glib::main_context_futures::TaskSource::dispatch
  12: g_main_context_dispatch
  13: <unknown>
  14: g_main_context_iteration
  15: g_application_run
  16: <O as gio::application::ApplicationExtManual>::run
  17: async_ui_gtk::mount::mount
  18: gtk_hackernews::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I'm using rust nightly on popos 22.04 for this particular test.

omac777 commented 1 year ago

I tweaked it so that it doesn't panic, but it left-justifies the item content rather than centering it afterwards.

 $ git diff
diff --git a/async_ui_gtk/src/components/list.rs b/async_ui_gtk/src/components/list.rs
index 0e818a8..5fe26aa 100644
--- a/async_ui_gtk/src/components/list.rs
+++ b/async_ui_gtk/src/components/list.rs
@@ -236,9 +236,11 @@ pub async fn list<'c, T: Clone, F: IntoFuture<Output = ()>>(
                         match keys_map.get_mut(&key.get_key_id()) {
                             Some(ItemAndTask { task, .. }) if task.is_some() => {
                                 *task = None;
-                            }
-                            Some(ItemAndTask { .. }) => panic!("invalid unbind"),
-                            _ => {}
+                            },
+                            Some(ItemAndTask { .. }) => {
+                               //panic!("invalid unbind");
+                           },
+                            _ => {},
                         }
                     }
                 }
wishawa commented 1 year ago

I did some more testing and turns out my list implementation is very buggy. This is mainly due to GTK expecting their API to be used in a different way from what we're doing. GTK is very OOP focused after all...

Your fix prevents the most common crash cause, but the way it moves elements to the left shows that something is still wrong under the hood. My suspicion is GTK thinks there are more things in each row and so it push stuff to the left to make space.

Since this current implementation is broken, I've changed the list component to be a simple box with a scroll window instead. (in commit 85eb8e005867d13a8ca8be29637b62c353759a97) In the future we can try to add a proper API for using GTK ListView.