Job state handling cleanup

src/app.rs

@@ -17,9 +17,9 @@ use time::{OffsetDateTime, UtcOffset};
 use crate::{
     config::{build_globset, load_project_config, ProjectUnit, ProjectUnitNode, CONFIG_FILENAMES},
     jobs::{
-        check_update::{queue_check_update, CheckUpdateResult},
+        check_update::{start_check_update, CheckUpdateResult},
-        objdiff::{queue_build, BuildStatus, ObjDiffResult},
+        objdiff::{start_build, BuildStatus, ObjDiffResult},
-        Job, JobResult, JobState, JobStatus,
+        Job, JobQueue, JobResult, JobStatus,
     },
     views::{
         appearance::{appearance_window, DEFAULT_COLOR_ROTATION},
@@ -107,7 +107,7 @@ pub struct SymbolReference {
 #[serde(default)]
 pub struct ViewState {
     #[serde(skip)]
-    pub jobs: Vec<JobState>,
+    pub jobs: JobQueue,
     #[serde(skip)]
     pub build: Option<Box<ObjDiffResult>>,
     #[serde(skip)]
@@ -142,7 +142,7 @@ pub struct ViewState {
 impl Default for ViewState {
     fn default() -> Self {
         Self {
-            jobs: vec![],
+            jobs: Default::default(),
             build: None,
             highlighted_symbol: None,
             selected_symbol: None,
@@ -380,7 +380,7 @@ impl eframe::App for App {
                 if function_diff_ui(ui, view_state) {
                     view_state
                         .jobs
-                        .push(queue_build(config.clone(), view_state.diff_config.clone()));
+                        .push(start_build(config.clone(), view_state.diff_config.clone()));
                 }
             });
         } else if view_state.current_view == View::DataDiff
@@ -390,7 +390,7 @@ impl eframe::App for App {
                 if data_diff_ui(ui, view_state) {
                     view_state
                         .jobs
-                        .push(queue_build(config.clone(), view_state.diff_config.clone()));
+                        .push(start_build(config.clone(), view_state.diff_config.clone()));
                 }
             });
         } else {
@@ -410,14 +410,7 @@ impl eframe::App for App {
 
         // Windows + request_repaint_after breaks dialogs:
         // https://github.com/emilk/egui/issues/2003
-        if cfg!(windows)
+        if cfg!(windows) || view_state.jobs.any_running() {
-            || view_state.jobs.iter().any(|job| {
-                if let Some(handle) = &job.handle {
-                    return !handle.is_finished();
-                }
-                false
-            })
-        {
             ctx.request_repaint();
         } else {
             ctx.request_repaint_after(Duration::from_millis(100));
@@ -433,14 +426,8 @@ impl eframe::App for App {
     }
 
     fn post_rendering(&mut self, _window_size_px: [u32; 2], _frame: &eframe::Frame) {
-        for job in &mut self.view_state.jobs {
+        for (job, result) in self.view_state.jobs.iter_finished() {
-            let Some(handle) = &job.handle else {
+            match result {
-                continue;
-            };
-            if !handle.is_finished() {
-                continue;
-            }
-            match job.handle.take().unwrap().join() {
                 Ok(result) => {
                     log::info!("Job {} finished", job.id);
                     match result {
@@ -496,26 +483,12 @@ impl eframe::App for App {
                 }
             }
         }
-        if self.view_state.jobs.iter().any(|v| v.should_remove) {
+        self.view_state.jobs.clear_finished();
-            let mut i = 0;
-            while i < self.view_state.jobs.len() {
-                let job = &self.view_state.jobs[i];
-                if job.should_remove
-                    && job.handle.is_none()
-                    && job.status.read().unwrap().error.is_none()
-                {
-                    self.view_state.jobs.remove(i);
-                } else {
-                    i += 1;
-                }
-            }
-        }
 
         if let Ok(mut config) = self.config.write() {
             let config = &mut *config;
 
-            if self.config_modified.load(Ordering::Relaxed) {
+            if self.config_modified.swap(false, Ordering::Relaxed) {
-                self.config_modified.store(false, Ordering::Relaxed);
                 config.config_change = true;
             }
 
@@ -551,23 +524,17 @@ impl eframe::App for App {
                 }
             }
 
-            if config.obj_path.is_some() && self.modified.load(Ordering::Relaxed) {
+            if config.obj_path.is_some()
-                if !self
+                && self.modified.swap(false, Ordering::Relaxed)
-                    .view_state
+                && !self.view_state.jobs.is_running(Job::ObjDiff)
+            {
+                self.view_state
                     .jobs
-                    .iter()
+                    .push(start_build(self.config.clone(), self.view_state.diff_config.clone()));
-                    .any(|j| j.job_type == Job::ObjDiff && j.handle.is_some())
-                {
-                    self.view_state.jobs.push(queue_build(
-                        self.config.clone(),
-                        self.view_state.diff_config.clone(),
-                    ));
-                }
-                self.modified.store(false, Ordering::Relaxed);
             }
 
             if config.queue_update_check {
-                self.view_state.jobs.push(queue_check_update());
+                self.view_state.jobs.push(start_check_update());
                 config.queue_update_check = false;
             }
         }

src/jobs/bindiff.rs

@@ -5,7 +5,7 @@ use anyhow::{Error, Result};
 use crate::{
     app::{AppConfig, DiffConfig},
     diff::diff_objs,
-    jobs::{queue_job, update_status, Job, JobResult, JobState, Status},
+    jobs::{start_job, update_status, Job, JobResult, JobState, Status},
     obj::{elf, ObjInfo},
 };
 
@@ -37,8 +37,8 @@ fn run_build(
     Ok(Box::new(BinDiffResult { first_obj: left_obj, second_obj: right_obj }))
 }
 
-pub fn queue_bindiff(config: Arc<RwLock<AppConfig>>) -> JobState {
+pub fn start_bindiff(config: Arc<RwLock<AppConfig>>) -> JobState {
-    queue_job("Binary diff", Job::BinDiff, move |status, cancel| {
+    start_job("Binary diff", Job::BinDiff, move |status, cancel| {
         run_build(status, cancel, config).map(JobResult::BinDiff)
     })
 }

src/jobs/check_update.rs

@@ -4,7 +4,7 @@ use anyhow::{Context, Result};
 use self_update::{cargo_crate_version, update::Release};
 
 use crate::{
-    jobs::{queue_job, update_status, Job, JobResult, JobState, Status},
+    jobs::{start_job, update_status, Job, JobResult, JobState, Status},
     update::{build_updater, BIN_NAME},
 };
 
@@ -26,8 +26,8 @@ fn run_check_update(status: &Status, cancel: Receiver<()>) -> Result<Box<CheckUp
     Ok(Box::new(CheckUpdateResult { update_available, latest_release, found_binary }))
 }
 
-pub fn queue_check_update() -> JobState {
+pub fn start_check_update() -> JobState {
-    queue_job("Check for updates", Job::CheckUpdate, move |status, cancel| {
+    start_job("Check for updates", Job::CheckUpdate, move |status, cancel| {
         run_check_update(status, cancel).map(JobResult::CheckUpdate)
     })
 }

src/jobs/mod.rs

@@ -27,14 +27,72 @@ pub enum Job {
     Update,
 }
 pub static JOB_ID: AtomicUsize = AtomicUsize::new(0);
+
+#[derive(Default)]
+pub struct JobQueue {
+    pub jobs: Vec<JobState>,
+}
+
+impl JobQueue {
+    /// Adds a job to the queue.
+    pub fn push(&mut self, state: JobState) { self.jobs.push(state); }
+
+    /// Returns whether a job of the given kind is running.
+    pub fn is_running(&self, kind: Job) -> bool {
+        self.jobs.iter().any(|j| j.kind == kind && j.handle.is_some())
+    }
+
+    /// Returns whether any job is running.
+    pub fn any_running(&self) -> bool {
+        self.jobs.iter().any(|job| {
+            if let Some(handle) = &job.handle {
+                return !handle.is_finished();
+            }
+            false
+        })
+    }
+
+    /// Iterates over all jobs mutably.
+    pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut JobState> + '_ { self.jobs.iter_mut() }
+
+    /// Iterates over all finished jobs, returning the job state and the result.
+    pub fn iter_finished(
+        &mut self,
+    ) -> impl Iterator<Item = (&mut JobState, std::thread::Result<JobResult>)> + '_ {
+        self.jobs.iter_mut().filter_map(|job| {
+            if let Some(handle) = &job.handle {
+                if !handle.is_finished() {
+                    return None;
+                }
+                let result = job.handle.take().unwrap().join();
+                return Some((job, result));
+            }
+            None
+        })
+    }
+
+    /// Clears all finished jobs.
+    pub fn clear_finished(&mut self) {
+        self.jobs.retain(|job| {
+            !(job.should_remove
+                && job.handle.is_none()
+                && job.status.read().unwrap().error.is_none())
+        });
+    }
+
+    /// Removes a job from the queue given its ID.
+    pub fn remove(&mut self, id: usize) { self.jobs.retain(|job| job.id != id); }
+}
+
 pub struct JobState {
     pub id: usize,
-    pub job_type: Job,
+    pub kind: Job,
     pub handle: Option<JoinHandle<JobResult>>,
     pub status: Arc<RwLock<JobStatus>>,
     pub cancel: Sender<()>,
     pub should_remove: bool,
 }
+
 #[derive(Default)]
 pub struct JobStatus {
     pub title: String,
@@ -43,6 +101,7 @@ pub struct JobStatus {
     pub status: String,
     pub error: Option<anyhow::Error>,
 }
+
 pub enum JobResult {
     None,
     ObjDiff(Box<ObjDiffResult>),
@@ -60,9 +119,9 @@ fn should_cancel(rx: &Receiver<()>) -> bool {
 
 type Status = Arc<RwLock<JobStatus>>;
 
-fn queue_job(
+fn start_job(
     title: &str,
-    job_type: Job,
+    kind: Job,
     run: impl FnOnce(&Status, Receiver<()>) -> Result<JobResult> + Send + 'static,
 ) -> JobState {
     let status = Arc::new(RwLock::new(JobStatus {
@@ -89,7 +148,7 @@ fn queue_job(
     log::info!("Started job {}", id);
     JobState {
         id,
-        job_type,
+        kind,
         handle: Some(handle),
         status: status_clone,
         cancel: tx,

src/jobs/objdiff.rs

@@ -11,7 +11,7 @@ use time::OffsetDateTime;
 use crate::{
     app::{AppConfig, DiffConfig},
     diff::diff_objs,
-    jobs::{queue_job, update_status, Job, JobResult, JobState, Status},
+    jobs::{start_job, update_status, Job, JobResult, JobState, Status},
     obj::{elf, ObjInfo},
 };
 
@@ -136,8 +136,8 @@ fn run_build(
     Ok(Box::new(ObjDiffResult { first_status, second_status, first_obj, second_obj, time }))
 }
 
-pub fn queue_build(config: Arc<RwLock<AppConfig>>, diff_config: DiffConfig) -> JobState {
+pub fn start_build(config: Arc<RwLock<AppConfig>>, diff_config: DiffConfig) -> JobState {
-    queue_job("Object diff", Job::ObjDiff, move |status, cancel| {
+    start_job("Object diff", Job::ObjDiff, move |status, cancel| {
         run_build(status, cancel, config, diff_config).map(JobResult::ObjDiff)
     })
 }

src/jobs/update.rs

@@ -9,7 +9,7 @@ use anyhow::{Context, Result};
 use const_format::formatcp;
 
 use crate::{
-    jobs::{queue_job, update_status, Job, JobResult, JobState, Status},
+    jobs::{start_job, update_status, Job, JobResult, JobState, Status},
     update::{build_updater, BIN_NAME},
 };
 
@@ -53,8 +53,8 @@ fn run_update(status: &Status, cancel: Receiver<()>) -> Result<Box<UpdateResult>
     Ok(Box::from(UpdateResult { exe_path: target_file }))
 }
 
-pub fn queue_update() -> JobState {
+pub fn start_update() -> JobState {
-    queue_job("Update app", Job::Update, move |status, cancel| {
+    start_job("Update app", Job::Update, move |status, cancel| {
         run_update(status, cancel).map(JobResult::Update)
     })
 }

src/views/config.rs

@@ -18,7 +18,7 @@ use self_update::cargo_crate_version;
 use crate::{
     app::{AppConfig, DiffKind, ViewConfig, ViewState},
     config::{ProjectUnit, ProjectUnitNode},
-    jobs::{bindiff::queue_bindiff, objdiff::queue_build, update::queue_update},
+    jobs::{bindiff::start_bindiff, objdiff::start_build, update::start_update},
     update::RELEASE_URL,
 };
 
@@ -101,7 +101,7 @@ pub fn config_ui(ui: &mut egui::Ui, config: &Arc<RwLock<AppConfig>>, view_state:
                         )
                         .clicked()
                 {
-                    view_state.jobs.push(queue_update());
+                    view_state.jobs.push(start_update());
                 }
                 if ui
                     .button("Manual")
@@ -190,7 +190,7 @@ pub fn config_ui(ui: &mut egui::Ui, config: &Arc<RwLock<AppConfig>>, view_state:
                 build = true;
             }
             if build {
-                view_state.jobs.push(queue_build(config.clone(), view_state.diff_config.clone()));
+                view_state.jobs.push(start_build(config.clone(), view_state.diff_config.clone()));
             }
         }
     } else if view_state.diff_kind == DiffKind::WholeBinary {
@@ -218,7 +218,7 @@ pub fn config_ui(ui: &mut egui::Ui, config: &Arc<RwLock<AppConfig>>, view_state:
 
         if let (Some(_), Some(_)) = (left_obj, right_obj) {
             if ui.button("Build").clicked() {
-                view_state.jobs.push(queue_bindiff(config.clone()));
+                view_state.jobs.push(start_bindiff(config.clone()));
             }
         }
     }

src/views/data_diff.rs

@@ -212,7 +212,7 @@ pub fn data_diff_ui(ui: &mut egui::Ui, view_state: &mut ViewState) -> bool {
                         ui.scope(|ui| {
                             ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace);
                             ui.style_mut().wrap = Some(false);
-                            if view_state.jobs.iter().any(|job| job.job_type == Job::ObjDiff) {
+                            if view_state.jobs.is_running(Job::ObjDiff) {
                                 ui.colored_label(view_state.view_config.replace_color, "Building…");
                             } else {
                                 ui.label("Last built:");

src/views/function_diff.rs

@@ -445,7 +445,7 @@ pub fn function_diff_ui(ui: &mut egui::Ui, view_state: &mut ViewState) -> bool {
                         ui.scope(|ui| {
                             ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace);
                             ui.style_mut().wrap = Some(false);
-                            if view_state.jobs.iter().any(|job| job.job_type == Job::ObjDiff) {
+                            if view_state.jobs.is_running(Job::ObjDiff) {
                                 ui.colored_label(view_state.view_config.replace_color, "Building…");
                             } else {
                                 ui.label("Last built:");

src/views/jobs.rs

@@ -6,7 +6,7 @@ pub fn jobs_ui(ui: &mut egui::Ui, view_state: &mut ViewState) {
     ui.label("Jobs");
 
     let mut remove_job: Option<usize> = None;
-    for (idx, job) in view_state.jobs.iter_mut().enumerate() {
+    for job in view_state.jobs.iter_mut() {
         let Ok(status) = job.status.read() else {
             continue;
         };
@@ -20,7 +20,7 @@ pub fn jobs_ui(ui: &mut egui::Ui, view_state: &mut ViewState) {
                             log::error!("Failed to cancel job: {e:?}");
                         }
                     } else {
-                        remove_job = Some(idx);
+                        remove_job = Some(job.id);
                     }
                 }
             });