Repository: pop-os/popsicle (language: rust)

Issue:
Hash freezes the application
**Reproduce**
1. Open Popsicle
2. Choose Image
3. Change Hash type either to SHA256 or MD5

**Outcome:** Until it generates the Hash, the whole application freezes. We cant even move the window

4. Assuming we previously had selected SHA256, **select MD5 now**
5. Application freezes again
6. Reselect SHA256

**Outcome:** Application again freezes, and i guess it tries to re-generate the hash, when it already knows it from previous generation

The "freeze" is for 3-4 seconds, so it is not unnoticeable 



Discussion / hints:
Will try to resolve within the next 24 hours. This part hasn't been made asynchronous yet.

Fix patch:
diff --git a/gtk/src/app/hash.rs b/gtk/src/app/hash.rs
deleted file mode 100644
index 758ec7f..0000000
--- a/gtk/src/app/hash.rs
+++ /dev/null
@@ -1,27 +0,0 @@
-use digest::{Digest, Input};
-use gtk::{self, EntryExt};
-use md5::Md5;
-use sha3::Sha3_256;
-
-fn md5_hasher(data: &[u8]) -> String {
-    let mut hasher = Md5::default();
-    hasher.process(data);
-    format!("{:x}", hasher.result())
-}
-
-fn sha256_hasher(data: &[u8]) -> String {
-    let mut hasher = Sha3_256::default();
-    hasher.process(data);
-    format!("{:x}", hasher.result())
-}
-
-pub(crate) fn set(entry: &gtk::Entry, hash: &str, data: &[u8]) {
-    let hash = match hash {
-        "Type" => return,
-        "SHA256" => sha256_hasher(data),
-        "MD5" => md5_hasher(data),
-        _ => unimplemented!(),
-    };
-
-    entry.get_buffer().set_text(&hash);
-}
diff --git a/gtk/src/app/mod.rs b/gtk/src/app/mod.rs
index cfdc651..756b6d2 100644
--- a/gtk/src/app/mod.rs
+++ b/gtk/src/app/mod.rs
@@ -1,6 +1,5 @@
 mod content;
 mod dialogs;
-mod hash;
 mod header;
 mod misc;
 mod state;
diff --git a/gtk/src/app/state.rs b/gtk/src/app/state.rs
index 18ff86f..36ef5d0 100644
--- a/gtk/src/app/state.rs
+++ b/gtk/src/app/state.rs
@@ -1,7 +1,8 @@
 use super::super::BlockDevice;
-use super::{hash, App, OpenDialog};
+use super::{App, OpenDialog};
 
-use image::BufferingData;
+use hash::HashState;
+use image::{self, BufferingData};
 use std::cell::{Cell, RefCell};
 use std::mem;
 use std::ops::{Deref, DerefMut};
@@ -25,6 +26,8 @@ pub struct State {
     pub buffer: Arc<BufferingData>,
     /// Contains a list of devices detected, and their check buttons.
     pub devices: Mutex<Vec<(String, CheckButton)>>,
+    /// Manages the state of hash requests
+    pub(crate) hash: Arc<HashState>,
     /// Useful for storing the size of the image that was loaded.
     pub image_length: Cell<usize>,
     /// Signals to load a new disk image into the `buffer` field.
@@ -46,6 +49,7 @@ impl State {
         State {
             bars: RefCell::new(Vec::new()),
             devices: Mutex::new(Vec::new()),
+            hash: Arc::new(HashState::new()),
             task_handles: Mutex::new(Vec::new()),
             tasks: Mutex::new(Vec::new()),
             view: Cell::new(0),
@@ -65,14 +69,14 @@ pub struct FlashTask {
 
 macro_rules! try_or_error {
     (
-        $act: expr,
-        $view: expr,
-        $back: expr,
-        $next: expr,
-        $stack: ident,
-        $error: ident,
-        $msg: expr,
-        $val: expr
+        $act:expr,
+        $view:expr,
+        $back:expr,
+        $next:expr,
+        $stack:ident,
+        $error:ident,
+        $msg:expr,
+        $val:expr
     ) => {
         match $act {
             Ok(value) => value,
@@ -152,19 +156,40 @@ impl Connect for App {
     fn connect_hash_generator(&self) {
         let state = self.state.clone();
         let hash_label = self.content.image_view.hash_label.clone();
-        self.content.image_view.hash.connect_changed(move |hash| {
-            if state.buffer.state.load(Ordering::SeqCst) == 0b010 {
-                if let Ok(lock) = state.buffer.data.lock() {
-                    let (_, ref data) = *lock;
-                    hash_label.set_icon_from_icon_name(EntryIconPosition::Primary, "gnome-spinner");
-                    hash_label.set_icon_sensitive(EntryIconPosition::Primary, true);
-                    if let Some(text) = hash.get_active_text() {
-                        hash::set(&hash_label, text.as_str(), data);
+        self.content
+            .image_view
+            .hash
+            .connect_changed(move |hash_kind| {
+                let hash_kind = match hash_kind.get_active() {
+                    1 => Some("SHA256"),
+                    2 => Some("MD5"),
+                    _ => None,
+                };
+
+                if let Some(hash_kind) = hash_kind {
+                    if hash_kind != "Type" {
+                        let hash = state.hash.clone();
+                        thread::spawn(move || {
+                            while hash.is_busy() {
+                                thread::yield_now();
+                            }
+
+                            hash.request(hash_kind);
+                        });
+
+                        let hash = state.hash.clone();
+                        let hash_label = hash_label.clone();
+                        gtk::timeout_add(1000, move || {
+                            if !hash.is_ready() {
+                                return Continue(true);
+                            }
+
+                            hash_label.set_text(hash.obtain().as_str());
+                            Continue(false)
+                        });
                     }
-                    hash_label.set_icon_sensitive(EntryIconPosition::Primary, false);
                 }
-            }
-        });
+            });
     }
 
     fn connect_back_button(&self) {
@@ -210,7 +235,7 @@ impl Connect for App {
 
         next.connect_clicked(move |next| {
             let device_list = &state.devices;
-            state.buffer.state.store(0b1000, Ordering::SeqCst);
+            state.buffer.state.store(image::SLEEPING, Ordering::SeqCst);
             let (_, ref mut image_data) = *try_or_error!(
                 state.buffer.data.lock(),
                 state.view,
@@ -311,7 +336,8 @@ impl Connect for App {
                         "device list mutex lock failure",
                         ()
                     );
-                    let devs = device_list.iter()
+                    let devs = device_list
+                        .iter()
                         .filter(|x| x.1.get_active())
                         .map(|x| x.0.clone());
 
@@ -409,9 +435,7 @@ impl Connect for App {
                         };
 
                         label.set_justify(Justification::Right);
-                        label
-                            .get_style_context()
-                            .map(|c| c.add_class("bold"));
+                        label.get_style_context().map(|c| c.add_class("bold"));
                         let bar_label = Label::new("");
                         bar_label.set_halign(Align::Center);
                         let bar_container = Box::new(Orientation::Vertical, 0);
@@ -507,16 +531,21 @@ impl Connect for App {
 
             // Ensure that the image has been loaded before continuing.
             match state.buffer.state.load(Ordering::SeqCst) {
-                0b0000 => {
+                0 => {
                     return Continue(true);
                 }
-                0b0001 => {
+                image::PROCESSING => {
                     chooser_container.set_visible_child_name("loader");
                     next.set_sensitive(false);
                     return Continue(true);
                 }
-                0b0010 => {
+                image::COMPLETED => {
                     chooser_container.set_visible_child_name("chooser");
+
+                    if state.hash.is_busy() {
+                        return Continue(true);
+                    }
+
                     let (ref path, ref data) = *try_or_error!(
                         state.buffer.data.lock(),
                         state.view,
@@ -533,12 +562,12 @@ impl Connect for App {
                         .to_string_lossy());
                     image_length.set(data.len());
                 }
-                0b0100 => {
+                image::ERRORED => {
                     chooser_container.set_visible_child_name("chooser");
                     next.set_sensitive(false);
                     return Continue(true);
                 }
-                0b1000 => (),
+                image::SLEEPING => (),
                 _ => unreachable!(),
             }
 
diff --git a/gtk/src/hash.rs b/gtk/src/hash.rs
new file mode 100644
index 0000000..052d7f9
--- /dev/null
+++ b/gtk/src/hash.rs
@@ -0,0 +1,121 @@
+use digest::{Digest, Input};
+use image::{self, BufferingData};
+use md5::Md5;
+use sha3::Sha3_256;
+use std::collections::BTreeMap;
+use std::path::PathBuf;
+use std::sync::Mutex;
+use std::sync::atomic::{AtomicUsize, Ordering};
+use std::thread;
+
+pub(crate) const SLEEPING: usize = 0;
+pub(crate) const PROCESSING: usize = 1;
+pub(crate) const COMPLETED: usize = 2;
+pub(crate) const INTERRUPT: usize = 3;
+
+pub(crate) struct HashState {
+    // K = Variant, V = Hashed Value
+    pub(crate) data: Mutex<HashData>,
+    // Defines if a hash is being generated.
+    pub(crate) state: AtomicUsize,
+}
+
+impl HashState {
+    pub(crate) fn new() -> HashState {
+        HashState {
+            data:  Mutex::new(HashData::new()),
+            state: SLEEPING.into(),
+        }
+    }
+
+    /// If it's true, it's not sleeping.
+    pub(crate) fn is_busy(&self) -> bool { self.state.load(Ordering::SeqCst) != SLEEPING }
+
+    pub(crate) fn is_ready(&self) -> bool { self.state.load(Ordering::SeqCst) == COMPLETED }
+
+    /// Requests for a new hash to be generated, if it hasn't been generated already.
+    pub(crate) fn request(&self, requested: &'static str) {
+        let mut data = self.data.lock().unwrap();
+        data.requested = requested;
+        self.state.store(INTERRUPT, Ordering::SeqCst);
+    }
+
+    /// Obtains the current hash stored within the state, if it exists.
+    pub(crate) fn obtain(&self) -> String {
+        let data = self.data.lock().unwrap();
+        let output = match data.store.get(data.requested) {
+            Some(hash) => hash.clone(),
+            None => "failed".into(),
+        };
+        self.state.store(SLEEPING, Ordering::SeqCst);
+        output
+    }
+}
+
+pub(crate) struct HashData {
+    requested: &'static str,
+    store:     BTreeMap<&'static str, String>,
+}
+
+impl HashData {
+    fn new() -> HashData {
+        HashData {
+            requested: "",
+            store:     BTreeMap::new(),
+        }
+    }
+}
+
+pub(crate) fn event_loop(buffer: &BufferingData, hash: &HashState) {
+    let mut last_image = PathBuf::new();
+
+    loop {
+        if hash.state.load(Ordering::SeqCst) == INTERRUPT {
+            hash.state.store(PROCESSING, Ordering::SeqCst);
+            let mut hash_data = hash.data.lock().unwrap();
+            while buffer.state.load(Ordering::SeqCst) != image::COMPLETED {
+                thread::yield_now();
+            }
+
+            let buffer_data = buffer.data.lock().unwrap();
+            let current_image = &buffer_data.0;
+            let image_data = &buffer_data.1;
+            let requested = hash_data.requested;
+            let same_image = &last_image != current_image.as_path();
+
+            if !same_image || !hash_data.store.contains_key(requested) {
+                if !same_image {
+                    hash_data.store.clear();
+                }
+
+                hash_data.store.insert(
+                    requested,
+                    match requested {
+                        "MD5" => md5_hasher(&image_data),
+                        "SHA256" => sha256_hasher(&image_data),
+                        _ => "Critical Error".into(),
+                    },
+                );
+            }
+
+            if !same_image {
+                last_image = current_image.to_path_buf();
+            }
+
+            hash.state.store(COMPLETED, Ordering::SeqCst);
+        }
+        thread::yield_now();
+    }
+}
+
+pub(crate) fn md5_hasher(data: &[u8]) -> String {
+    let mut hasher = Md5::default();
+    hasher.process(data);
+    format!("{:x}", hasher.result())
+}
+
+pub(crate) fn sha256_hasher(data: &[u8]) -> String {
+    let mut hasher = Sha3_256::default();
+    hasher.process(data);
+    format!("{:x}", hasher.result())
+}
diff --git a/gtk/src/image.rs b/gtk/src/image.rs
index c1ba96b..57b213f 100644
--- a/gtk/src/image.rs
+++ b/gtk/src/image.rs
@@ -22,12 +22,17 @@ impl BufferingData {
     }
 }
 
+pub const PROCESSING: usize = 1;
+pub const COMPLETED: usize = 2;
+pub const ERRORED: usize = 3;
+pub const SLEEPING: usize = 4;
+
 /// An event loop that is meant to be run in a background thread, receiving image paths
 /// to load, and buffering those images into the application's shared `BufferingData`
 /// field.
-pub fn image_load_event_loop(path_receiver: Receiver<PathBuf>, buffer: &BufferingData) {
+pub fn event_loop(path_receiver: Receiver<PathBuf>, buffer: &BufferingData) {
     while let Ok(path) = path_receiver.recv() {
-        buffer.state.store(0b1, Ordering::SeqCst);
+        buffer.state.store(PROCESSING, Ordering::SeqCst);
         let (ref mut name, ref mut data) = *buffer
             .data
             .lock()
@@ -35,11 +40,11 @@ pub fn image_load_event_loop(path_receiver: Receiver<PathBuf>, buffer: &Bufferin
         match load_image(&path, data) {
             Ok(_) => {
                 *name = path;
-                buffer.state.store(0b10, Ordering::SeqCst);
+                buffer.state.store(COMPLETED, Ordering::SeqCst);
             }
             Err(why) => {
                 eprintln!("popsicle-gtk: image loading error: {}", why);
-                buffer.state.store(0b100, Ordering::SeqCst);
+                buffer.state.store(ERRORED, Ordering::SeqCst);
             }
         }
     }
diff --git a/gtk/src/main.rs b/gtk/src/main.rs
index 1db7695..74ef715 100644
--- a/gtk/src/main.rs
+++ b/gtk/src/main.rs
@@ -9,6 +9,7 @@ extern crate sha3;
 
 mod app;
 mod block;
+mod hash;
 mod image;
 
 use app::{App, Connect};
@@ -34,7 +35,11 @@ fn main() {
 
     {
         let buffer = app.state.buffer.clone();
-        thread::spawn(move || image::image_load_event_loop(receiver, &buffer));
+        thread::spawn(move || image::event_loop(receiver, &buffer));
+
+        let buffer = app.state.buffer.clone();
+        let hash = app.state.hash.clone();
+        thread::spawn(move || hash::event_loop(&buffer, &hash));
     }
 
     if let Some(iso_argument) = env::args().skip(1).next() {


Test patch: