Run all GTK dialogs on one thread

Per the GDK documentation: "GTK+, however, is not thread safe. You should only use GTK+ and GDK from the thread gtk_init() and gtk_main() were called on. This is usually referred to as the "main thread"." We were previously calling GTK functions from different threads--there was a mutex so that only one thread was calling GTK at any given time, but that's not sufficient--only the thread *that called gtk_init* is allowed to call GTK functions. To facilitate that, spin up a thread and initialize GTK on it, keeping it alive for the lifetime of the program. It's a bit unfortunate that we have to keep it around forever, but the GTK documentation seems to indicate that once you call gtk_init on a thread, you're stuck with that thread.
PolyMeilex · Sep 28, 2023 · 286226b · 286226b
1 parent 1c6823d
commit 286226b
Show file tree

Hide file tree

Showing 5 changed files with 129 additions and 177 deletions.
diff --git a/src/backend/gtk3/file_dialog.rs b/src/backend/gtk3/file_dialog.rs
@@ -4,7 +4,7 @@ use dialog_ffi::GtkFileDialog;
 
 use std::path::PathBuf;
 
-use super::utils::{gtk_init_check, GTK_MUTEX};
+use super::utils::GtkGlobalThread;
 use crate::backend::DialogFutureType;
 use crate::{FileDialog, FileHandle};
 
@@ -17,11 +17,7 @@ use super::gtk_future::GtkDialogFuture;
 use crate::backend::FilePickerDialogImpl;
 impl FilePickerDialogImpl for FileDialog {
     fn pick_file(self) -> Option<PathBuf> {
-        GTK_MUTEX.run_locked(|| {
-            if !gtk_init_check() {
-                return None;
-            };
-
+        GtkGlobalThread::instance().run_blocking(move || {
             let dialog = GtkFileDialog::build_pick_file(&self);
 
             if dialog.run() == gtk_sys::GTK_RESPONSE_ACCEPT {
@@ -33,11 +29,7 @@ impl FilePickerDialogImpl for FileDialog {
     }
 
     fn pick_files(self) -> Option<Vec<PathBuf>> {
-        GTK_MUTEX.run_locked(|| {
-            if !gtk_init_check() {
-                return None;
-            };
-
+        GtkGlobalThread::instance().run_blocking(move || {
             let dialog = GtkFileDialog::build_pick_files(&self);
 
             if dialog.run() == gtk_sys::GTK_RESPONSE_ACCEPT {
@@ -93,11 +85,7 @@ impl AsyncFilePickerDialogImpl for FileDialog {
 use crate::backend::FolderPickerDialogImpl;
 impl FolderPickerDialogImpl for FileDialog {
     fn pick_folder(self) -> Option<PathBuf> {
-        GTK_MUTEX.run_locked(|| {
-            if !gtk_init_check() {
-                return None;
-            };
-
+        GtkGlobalThread::instance().run_blocking(move || {
             let dialog = GtkFileDialog::build_pick_folder(&self);
 
             if dialog.run() == gtk_sys::GTK_RESPONSE_ACCEPT {
@@ -109,11 +97,7 @@ impl FolderPickerDialogImpl for FileDialog {
     }
 
     fn pick_folders(self) -> Option<Vec<PathBuf>> {
-        GTK_MUTEX.run_locked(|| {
-            if !gtk_init_check() {
-                return None;
-            };
-
+        GtkGlobalThread::instance().run_blocking(move || {
             let dialog = GtkFileDialog::build_pick_folders(&self);
 
             if dialog.run() == gtk_sys::GTK_RESPONSE_ACCEPT {
@@ -169,11 +153,7 @@ impl AsyncFolderPickerDialogImpl for FileDialog {
 use crate::backend::FileSaveDialogImpl;
 impl FileSaveDialogImpl for FileDialog {
     fn save_file(self) -> Option<PathBuf> {
-        GTK_MUTEX.run_locked(|| {
-            if !gtk_init_check() {
-                return None;
-            };
-
+        GtkGlobalThread::instance().run_blocking(move || {
             let dialog = GtkFileDialog::build_save_file(&self);
 
             if dialog.run() == gtk_sys::GTK_RESPONSE_ACCEPT {

diff --git a/src/backend/gtk3/file_dialog/dialog_ffi.rs b/src/backend/gtk3/file_dialog/dialog_ffi.rs
@@ -291,9 +291,7 @@ impl AsGtkDialog for GtkFileDialog {
 impl Drop for GtkFileDialog {
     fn drop(&mut self) {
         unsafe {
-            super::super::utils::wait_for_cleanup();
             gtk_sys::gtk_native_dialog_destroy(self.ptr as _);
-            super::super::utils::wait_for_cleanup();
         }
     }
 }
diff --git a/src/backend/gtk3/gtk_future.rs b/src/backend/gtk3/gtk_future.rs
@@ -1,14 +1,10 @@
-use super::utils::GTK_EVENT_HANDLER;
-use super::utils::GTK_MUTEX;
+use super::utils::GtkGlobalThread;
 
-use std::cell::RefCell;
 use std::pin::Pin;
-use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 
 use std::task::{Context, Poll, Waker};
 
-use super::utils::gtk_init_check;
 use super::AsGtkDialog;
 
 struct FutureState<R, D> {
@@ -39,49 +35,33 @@ impl<R: Default + 'static, D: AsGtkDialog + 'static> GtkDialogFuture<R, D> {
 
         {
             let state = state.clone();
-            std::thread::spawn(move || {
-                let request = Rc::new(RefCell::new(None));
+            let callback = {
+                let state = state.clone();
 
-                let callback = {
-                    let state = state.clone();
-                    let request = request.clone();
-
-                    // Callbacks are called by GTK_EVENT_HANDLER so the GTK_MUTEX is allready locked, no need to worry about that here
-                    move |res_id| {
-                        let mut state = state.lock().unwrap();
-
-                        if let Some(mut dialog) = state.dialog.take() {
-                            state.data = Some(cb(&mut dialog, res_id));
-                        }
-
-                        // Drop the request
-                        request.borrow_mut().take();
+                move |res_id| {
+                    let mut state = state.lock().unwrap();
 
-                        if let Some(waker) = state.waker.take() {
-                            waker.wake();
-                        }
+                    if let Some(mut dialog) = state.dialog.take() {
+                        state.data = Some(cb(&mut dialog, res_id));
                     }
-                };
 
-                GTK_MUTEX.run_locked(|| {
-                    let mut state = state.lock().unwrap();
-                    if gtk_init_check() {
-                        state.dialog = Some(build());
+                    if let Some(waker) = state.waker.take() {
+                        waker.wake();
                     }
+                }
+            };
 
-                    if let Some(dialog) = &state.dialog {
-                        unsafe {
-                            dialog.show();
+            GtkGlobalThread::instance().run(move || {
+                let mut state = state.lock().unwrap();
+                state.dialog = Some(build());
 
-                            let ptr = dialog.gtk_dialog_ptr();
-                            connect_response(ptr as *mut _, callback);
-                        }
-                    } else {
-                        state.data = Some(Default::default());
-                    }
-                });
+                unsafe {
+                    let dialog = state.dialog.as_ref().unwrap();
+                    dialog.show();
 
-                request.replace(Some(GTK_EVENT_HANDLER.request_iteration_start()));
+                    let ptr = dialog.gtk_dialog_ptr();
+                    connect_response(ptr as *mut _, callback);
+                }
             });
         }
 

diff --git a/src/backend/gtk3/message_dialog.rs b/src/backend/gtk3/message_dialog.rs
@@ -2,7 +2,7 @@ use std::ffi::CString;
 use std::ptr;
 
 use super::gtk_future::GtkDialogFuture;
-use super::utils::wait_for_cleanup;
+use super::utils::GtkGlobalThread;
 use super::AsGtkDialog;
 
 use crate::message_dialog::{MessageButtons, MessageDialog, MessageLevel};
@@ -15,8 +15,6 @@ pub struct GtkMessageDialog {
 
 impl GtkMessageDialog {
     pub fn new(opt: MessageDialog) -> Self {
-        super::utils::gtk_init_check();
-
         let level = match opt.level {
             MessageLevel::Info => gtk_sys::GTK_MESSAGE_INFO,
             MessageLevel::Warning => gtk_sys::GTK_MESSAGE_WARNING,
@@ -174,9 +172,7 @@ unsafe fn set_child_labels_selectable(dialog: *mut gtk_sys::GtkDialog) {
 impl Drop for GtkMessageDialog {
     fn drop(&mut self) {
         unsafe {
-            wait_for_cleanup();
             gtk_sys::gtk_widget_destroy(self.ptr as *mut _);
-            wait_for_cleanup();
         }
     }
 }
@@ -194,8 +190,10 @@ use crate::backend::MessageDialogImpl;
 
 impl MessageDialogImpl for MessageDialog {
     fn show(self) -> MessageDialogResult {
-        let dialog = GtkMessageDialog::new(self);
-        dialog.run()
+        GtkGlobalThread::instance().run_blocking(move || {
+            let dialog = GtkMessageDialog::new(self);
+            dialog.run()
+        })
     }
 }