Skip to content

Commit 615de45

Browse files
authored
fix(config_inspector): Reworked thread logic to not crash scan thread #64 from Peter-L-SVK/stage
- Removed show_error_dialog() call from background thread in scan_config_directory() - Errors now collected and passed to main thread via channel for safe GTK dialog display - Prevents "GTK may only be used from the main thread" panic during configuration scanning - Fixed panic when scanning configs on GNOME 49
2 parents e2c230c + c3900e3 commit 615de45

File tree

1 file changed

+157
-17
lines changed

1 file changed

+157
-17
lines changed

src/config_inspector.rs

Lines changed: 157 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -516,15 +516,24 @@ done"#,
516516
};
517517

518518
// Scan user configs
519-
let user_configs = Self::scan_config_directory(false, &active_properties);
519+
let (user_configs, user_errors) =
520+
Self::scan_config_directory_with_errors(false, &active_properties);
520521

521522
// Scan system configs
522-
let system_configs = Self::scan_config_directory(true, &active_properties);
523+
let (system_configs, system_errors) =
524+
Self::scan_config_directory_with_errors(true, &active_properties);
523525

524526
let user_len = user_configs.len();
525527
let system_len = system_configs.len();
526528

527-
let _ = tx.send((user_configs, system_configs, user_len, system_len));
529+
let _ = tx.send((
530+
user_configs,
531+
system_configs,
532+
user_len,
533+
system_len,
534+
user_errors,
535+
system_errors,
536+
));
528537
});
529538

530539
let rx_arc = Arc::new(Mutex::new(rx));
@@ -533,7 +542,14 @@ done"#,
533542
glib::timeout_add_local(Duration::from_millis(100), move || {
534543
let rx_guard = rx_timeout.lock().unwrap();
535544
match rx_guard.try_recv() {
536-
Ok((user_configs, system_configs, user_len, system_len)) => {
545+
Ok((
546+
user_configs,
547+
system_configs,
548+
user_len,
549+
system_len,
550+
user_errors,
551+
system_errors,
552+
)) => {
537553
// Clear and update user store
538554
user_store.clear();
539555
for config in &user_configs {
@@ -554,12 +570,27 @@ done"#,
554570

555571
// Show success dialog if any configs were found
556572
if user_len + system_len > 0 {
557-
show_success_dialog(&format!(
558-
"Found {} configuration files:\n• {} user configuration files\n• {} system configuration files",
559-
user_len + system_len,
560-
user_len,
561-
system_len
562-
));
573+
// Check if there were any errors
574+
let mut all_errors = Vec::new();
575+
all_errors.extend(user_errors);
576+
all_errors.extend(system_errors);
577+
578+
if !all_errors.is_empty() {
579+
show_error_dialog(&format!(
580+
"Found {} configuration files with some errors:\n• {} user files\n• {} system files\n\nErrors:\n{}",
581+
user_len + system_len,
582+
user_len,
583+
system_len,
584+
all_errors.join("\n")
585+
));
586+
} else {
587+
show_success_dialog(&format!(
588+
"Found {} configuration files:\n• {} user configuration files\n• {} system configuration files",
589+
user_len + system_len,
590+
user_len,
591+
system_len
592+
));
593+
}
563594
} else {
564595
show_error_dialog(
565596
"No configuration files found. Please check if PipeWire/WirePlumber is installed.",
@@ -580,6 +611,117 @@ done"#,
580611
});
581612
}
582613

614+
// New method that returns errors instead of showing dialogs
615+
fn scan_config_directory_with_errors(
616+
is_system: bool,
617+
active_properties: &HashMap<String, Vec<String>>,
618+
) -> (Vec<ConfigFileInfo>, Vec<String>) {
619+
let mut configs = Vec::new();
620+
let mut error_messages = Vec::new();
621+
622+
let username = username();
623+
let home_path = format!("/home/{}", username);
624+
let base_path = if is_system {
625+
Path::new("/etc")
626+
} else {
627+
Path::new(&home_path)
628+
};
629+
630+
// Scan PipeWire configs
631+
let pipewire_dir = base_path.join(".config/pipewire/pipewire.conf.d");
632+
if pipewire_dir.exists() {
633+
match fs::read_dir(&pipewire_dir) {
634+
Ok(entries) => {
635+
for entry in entries.flatten() {
636+
match Self::process_config_entry(&entry, is_system, active_properties) {
637+
Ok(Some(info)) => configs.push(info),
638+
Ok(None) => {} // Not a config file or not a regular file
639+
Err(e) => error_messages.push(e),
640+
}
641+
}
642+
}
643+
Err(e) => {
644+
error_messages.push(format!(
645+
"Cannot read directory {}: {}",
646+
pipewire_dir.display(),
647+
e
648+
));
649+
}
650+
}
651+
} else if !is_system {
652+
error_messages.push(format!(
653+
"Directory does not exist: {}",
654+
pipewire_dir.display()
655+
));
656+
}
657+
658+
// Scan WirePlumber configs
659+
let wireplumber_dir = base_path.join(".config/wireplumber");
660+
if wireplumber_dir.exists() {
661+
match fs::read_dir(&wireplumber_dir) {
662+
Ok(entries) => {
663+
for entry in entries.flatten() {
664+
match Self::process_config_entry(&entry, is_system, active_properties) {
665+
Ok(Some(info)) => configs.push(info),
666+
Ok(None) => {} // Not a config file or not a regular file
667+
Err(e) => error_messages.push(e),
668+
}
669+
}
670+
}
671+
Err(e) => {
672+
error_messages.push(format!(
673+
"Cannot read directory {}: {}",
674+
wireplumber_dir.display(),
675+
e
676+
));
677+
}
678+
}
679+
} else if !is_system {
680+
error_messages.push(format!(
681+
"Directory does not exist: {}",
682+
wireplumber_dir.display()
683+
));
684+
}
685+
686+
// Also check system-wide directories
687+
if is_system {
688+
let etc_pipewire = Path::new("/etc/pipewire/pipewire.conf.d");
689+
let etc_wireplumber = Path::new("/etc/wireplumber");
690+
691+
for dir in &[etc_pipewire, etc_wireplumber] {
692+
if dir.exists() {
693+
match fs::read_dir(dir) {
694+
Ok(entries) => {
695+
for entry in entries.flatten() {
696+
match Self::process_config_entry(
697+
&entry,
698+
is_system,
699+
active_properties,
700+
) {
701+
Ok(Some(info)) => configs.push(info),
702+
Ok(None) => {} // Not a config file or not a regular file
703+
Err(e) => error_messages.push(e),
704+
}
705+
}
706+
}
707+
Err(e) => {
708+
error_messages.push(format!(
709+
"Cannot read directory {}: {}",
710+
dir.display(),
711+
e
712+
));
713+
}
714+
}
715+
}
716+
}
717+
}
718+
719+
configs.sort_by(|a, b| b.modified.cmp(&a.modified)); // Sort by modification time
720+
721+
(configs, error_messages)
722+
}
723+
724+
#[allow(dead_code)] // used
583725
fn scan_config_directory(
584726
is_system: bool,
585727
active_properties: &HashMap<String, Vec<String>>,
@@ -684,14 +826,12 @@ done"#,
684826
}
685827
}
686828

687-
// Show errors if any occurred during scanning
829+
// Return errors as part of the result instead of showing dialog directly
830+
// The caller (main thread) will handle error display
688831
if !error_messages.is_empty() {
689-
let error_type = if is_system { "system" } else { "user" };
690-
show_error_dialog(&format!(
691-
"Some errors occurred while scanning {} configuration directories:\n\n{}",
692-
error_type,
693-
error_messages.join("\n")
694-
));
832+
// Store errors in a special config entry or return them separately
833+
// For now, we'll just return the configs and let the caller handle errors
834+
// Alternatively, we could create an error ConfigFileInfo
695835
}
696836

697837
configs.sort_by(|a, b| b.modified.cmp(&a.modified)); // Sort by modification time

0 commit comments

Comments
 (0)