Skip to content

Conversation

@adderly
Copy link
Collaborator

@adderly adderly commented Nov 1, 2025

feat: Windows only one instance of the application
feat: Added working_dir for launching some executable
feat: Added run command when editing an app, for easily testing if configuration is ok (mainly when manually adding parameters for launch)
testing: Progress bar for UI feedback when processing actions

 - Text on remove button
feat: Added working_dir for launching some executable
testing: Progress bar for UI feedback when processing actions
@adderly adderly requested a review from middaysan November 1, 2025 01:48
@middaysan
Copy link
Owner

Hi! Thank you for your contribution to the project! I use AI to translate my review, sorry if it seems rude =)

But

Logic in common_os.rs mixes two approaches and adds brittle logic. Keep a single source of truth (named instance) and activate the existing window via a message. Drop process scanning.

Issues

  • Duplicated mechanisms. single_instance::SingleInstance already handles exclusivity atomically. Extra process-name scanning adds races and no guarantees.
  • Weak key. uniq_id = "sd" is not unique. Use a stable GUID, with Local\ or Global\ prefix as needed.
  • Process scanning is fragile. Breaks on renamed binaries, multiple copies, different users/sessions, and elevated processes. sysinfo snapshot may be stale. Comparing pname == os_string is type-fragile.
  • Focus by PID is incorrect. You need an HWND, not just a PID. This code can’t reliably focus the right window.
  • Unsafe globals. static mut SINGLE_INSTANCE_VAL with Once is unnecessary and unsafe. Use OnceCell<SingleInstance> or LazyLock. Also ensure the handle lives for the whole runtime.
  • Platform check bug. cfg!(target_os = "linux") { println!("Running on macOS!") } is wrong and suggests confusion in cross-platform handling.

Recommended approach

  1. Create a single instance with a strong key, e.g. Local\{GUID}-cpu-affinity-tool (use Global\ only if you truly need cross-session).
  2. If !is_single():
    • Register a custom message: RegisterWindowMessageW("CPU_AFFINITY_TOOL_ACTIVATE_{GUID}").
    • PostMessageW(HWND_BROADCAST, msg, 0, 0).
    • process::exit(0).
  3. In the main instance’s Win32 window proc, handle that message: ShowWindow(SW_RESTORE) then SetForegroundWindow.
  4. Keep the SingleInstance handle in a OnceCell to maintain ownership.

Something like that

static INSTANCE: once_cell::sync::OnceCell<single_instance::SingleInstance> = OnceCell::new();

const GUID: &str = "{2F3A9D6E-1B1A-4B7C-B1C7-3E0C9C7B3F11}";

fn activation_msg() -> u32 {
    unsafe { RegisterWindowMessageW(wide(&format!("CPU_AFFINITY_TOOL_ACTIVATE_{}", GUID)).as_ptr()) }
}

pub fn ensure_single_or_activate_and_exit() {
    let key = format!(r"Local\{}-cpu-affinity-tool", GUID);
    let inst = SingleInstance::new(&key).expect("single-instance init failed");
    if !inst.is_single() {
        unsafe { PostMessageW(HWND_BROADCAST, activation_msg(), 0, 0); }
        std::process::exit(0);
    }
    let _ = INSTANCE.set(inst);
}

Remove

  • sysinfo dependency and all process-name scanning.
  • static mut globals.
  • PID-based focusing.

This yields atomic single-instance behavior, correct activation, fewer dependencies, and cleaner cross-platform boundaries.

What do you think?

@adderly
Copy link
Collaborator Author

adderly commented Nov 6, 2025

@middaysan will check most of the observations you point out.

The process scanning will remain as a toolkit for later searching running processes and add them to a group from the running process.

Most of the work wasn't finished, i did prove of concept for new features, so expect better implementation when i update the this commits.

Is there a crossplatform way for doing the "RegisterWindowMessageW"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants