fix(windows): recycle menu command ids to keep them within 16 bits

[wonfen]

On Windows, menu items stop working once a process has created about 64.5k menus and items in total. The menu still opens, but clicks do nothing: no MenuEvent, no error.

Why: command ids come from a global counter that starts at 1000 and only ever goes up. Items are inserted with the full u32 id, but WM_COMMAND is matched on LOWORD(wParam), so once the counter passes 0xFFFF nothing matches anymore. ACCEL::cmd is 16-bit too.

64.5k sounds like a lot, but apps that rebuild their tray menu on every state change get there. We hit it in Clash Verge Rev: the tray lists proxy nodes and gets rebuilt on every switch, and after a few hours Quit and then every other tray item stopped working.

Fix: hand out fresh ids up to u16::MAX like today, then reuse ids from dropped menus and items, oldest first. Nothing changes until the first ~64.5k ids are used up.

A few notes:

• A WM_COMMAND still queued for a dropped item could in theory land on a new item that got its id. With oldest-first reuse, that takes tens of thousands of allocations between the click and its dispatch, so I think it's fine. Today every click past 0xFFFF is lost anyway.
• Nearby but not touched here: remove_at uses MF_BYCOMMAND, which also searches submenus, so it can remove the wrong row when an item sits in a menu and in one of its submenus. Can be fixed separately.
• Tauri 2 still depends on muda = "0.20". Any chance of a 0.20.x backport?

Testing

• New unit test churns ids across the u16::MAX boundary (kept small so it's quick under Miri).
• fmt, clippy (-D warnings) and tests pass on Windows 11.
• Repro below, muda only, showing the menu the way tray-icon does. On dev, the click on the item with id 65536 is lost, and after that every click is; with this PR they all go through.
• The same change on 0.20.0 fixed the tray in Clash Verge Rev.

Related: clash-verge-rev/clash-verge-rev#8159 (reproduced and verified). Probably the same bug: clash-verge-rev/clash-verge-rev#7131, clash-verge-rev/clash-verge-rev#6738, clash-verge-rev/clash-verge-rev#5520, clash-verge-rev/clash-verge-rev#6604, clash-verge-rev/clash-verge-rev#7095.

Repro

//! Windows repro for muda: once ~64.5k menus/items have been created in a process,
//! clicks on newly created items no longer produce a MenuEvent.
//!
//! Mirrors how tray-icon shows its menu: the popup is attached to a hidden window via
//! `attach_menu_subclass_for_hwnd`, shown with `TrackPopupMenu` (no `TPM_RETURNCMD`), and
//! the selection arrives as `WM_COMMAND`. A helper thread picks the item with the keyboard.
//!
//! windows-sys features: Win32_Foundation, Win32_System_LibraryLoader, Win32_System_Threading,
//! Win32_UI_Input_KeyboardAndMouse, Win32_UI_WindowsAndMessaging

use muda::{ContextMenu, Menu, MenuEvent, MenuItem};
use std::time::{Duration, Instant};
use windows_sys::Win32::Foundation::{FALSE, HWND};
use windows_sys::Win32::System::LibraryLoader::GetModuleHandleW;
use windows_sys::Win32::System::Threading::GetCurrentThreadId;
use windows_sys::Win32::UI::Input::KeyboardAndMouse::{VK_DOWN, VK_RETURN, VK_UP};
use windows_sys::Win32::UI::WindowsAndMessaging::*;

fn wide(s: &str) -> Vec<u16> {
    s.encode_utf16().chain(Some(0)).collect()
}

fn class_of(hwnd: HWND) -> String {
    let mut buf = [0u16; 64];
    let n = unsafe { GetClassNameW(hwnd, buf.as_mut_ptr(), buf.len() as i32) } as usize;
    String::from_utf16_lossy(&buf[..n])
}

/// The visible popup menu window owned by thread `tid`.
fn popup_of(tid: u32) -> Option<usize> {
    let mut hwnd: HWND = std::ptr::null_mut();
    loop {
        hwnd = unsafe { FindWindowExW(std::ptr::null_mut(), hwnd, std::ptr::null(), std::ptr::null()) };
        if hwnd.is_null() {
            return None;
        }
        if unsafe { GetWindowThreadProcessId(hwnd, std::ptr::null_mut()) } == tid
            && class_of(hwnd) == "#32768"
            && unsafe { IsWindowVisible(hwnd) } != FALSE
        {
            return Some(hwnd as usize);
        }
    }
}

fn pump(ms: u64) {
    let deadline = Instant::now() + Duration::from_millis(ms);
    let mut msg: MSG = unsafe { std::mem::zeroed() };
    while Instant::now() < deadline {
        unsafe {
            while PeekMessageW(&mut msg, std::ptr::null_mut(), 0, 0, PM_REMOVE) != 0 {
                TranslateMessage(&msg);
                DispatchMessageW(&msg);
            }
        }
        std::thread::sleep(Duration::from_millis(10));
    }
}

/// Show `menu` like tray-icon does, press `nav` then Enter, return the MenuEvent ids received.
fn click(hwnd: HWND, menu: &Menu, nav: u16) -> Vec<String> {
    while MenuEvent::receiver().try_recv().is_ok() {}
    let tid = unsafe { GetCurrentThreadId() };
    let helper = std::thread::spawn(move || {
        let start = Instant::now();
        while start.elapsed() < Duration::from_secs(3) {
            if let Some(popup) = popup_of(tid) {
                std::thread::sleep(Duration::from_millis(150));
                unsafe {
                    PostMessageW(popup as HWND, WM_KEYDOWN, nav as usize, 0);
                    PostMessageW(popup as HWND, WM_KEYDOWN, VK_RETURN as usize, 0);
                }
                return;
            }
            std::thread::sleep(Duration::from_millis(20));
        }
    });
    unsafe {
        SetForegroundWindow(hwnd);
        TrackPopupMenu(
            menu.hpopupmenu() as _,
            TPM_BOTTOMALIGN | TPM_LEFTALIGN,
            200,
            200,
            0,
            hwnd,
            std::ptr::null(),
        );
        PostMessageW(hwnd, WM_NULL, 0, 0);
    }
    helper.join().unwrap();
    pump(300);
    let mut ids = Vec::new();
    while let Ok(event) = MenuEvent::receiver().try_recv() {
        ids.push(event.id.0);
    }
    ids
}

fn scenario(hwnd: HWND, label: &str) -> u32 {
    let menu = Menu::new();
    let open = MenuItem::with_id("open", "Open", true, None);
    let exit = MenuItem::with_id("exit", "Exit", true, None);
    menu.append_items(&[&open, &exit]).unwrap();
    unsafe { menu.attach_menu_subclass_for_hwnd(hwnd as isize) };

    let hmenu = menu.hpopupmenu() as HMENU;
    let (open_id, exit_id) = unsafe { (GetMenuItemID(hmenu, 0), GetMenuItemID(hmenu, 1)) };
    println!("{label}: command ids Open={open_id} Exit={exit_id}");
    for (name, key) in [("Open", VK_DOWN), ("Exit", VK_UP)] {
        let ids = click(hwnd, &menu, key);
        println!("  click {name:<4} -> MenuEvent {:?}", ids);
    }
    unsafe { menu.detach_menu_subclass_from_hwnd(hwnd as isize) };
    exit_id
}

fn burn(n: u32) {
    for _ in 0..n {
        drop(MenuItem::new("", true, None));
    }
}

fn main() {
    let class = wide("muda_repro");
    let hwnd = unsafe {
        let wc = WNDCLASSW {
            lpfnWndProc: Some(DefWindowProcW),
            hInstance: GetModuleHandleW(std::ptr::null()),
            lpszClassName: class.as_ptr(),
            ..std::mem::zeroed()
        };
        RegisterClassW(&wc);
        CreateWindowExW(
            WS_EX_TOOLWINDOW,
            class.as_ptr(),
            std::ptr::null(),
            WS_OVERLAPPED,
            0,
            0,
            0,
            0,
            std::ptr::null_mut(),
            std::ptr::null_mut(),
            wc.hInstance,
            std::ptr::null(),
        )
    };
    assert!(!hwnd.is_null());

    let last = scenario(hwnd, "fresh process");
    // The next Menu, Open and Exit take 0xFFFE, 0xFFFF and 0x10000 without recycling.
    burn(0xFFFE - (last + 1));
    scenario(hwnd, "after ~64.5k allocations");
    burn(1_000);
    scenario(hwnd, "after ~65.5k allocations");
}

Output on dev (9d35d84):

fresh process: command ids Open=1001 Exit=1002
  click Open -> MenuEvent ["open"]
  click Exit -> MenuEvent ["exit"]
after ~64.5k allocations: command ids Open=65535 Exit=65536
  click Open -> MenuEvent ["open"]
  click Exit -> MenuEvent []
after ~65.5k allocations: command ids Open=66538 Exit=66539
  click Open -> MenuEvent []
  click Exit -> MenuEvent []

With this PR:

fresh process: command ids Open=1001 Exit=1002
  click Open -> MenuEvent ["open"]
  click Exit -> MenuEvent ["exit"]
after ~64.5k allocations: command ids Open=65535 Exit=1000
  click Open -> MenuEvent ["open"]
  click Exit -> MenuEvent ["exit"]
after ~65.5k allocations: command ids Open=2002 Exit=2003
  click Open -> MenuEvent ["open"]
  click Exit -> MenuEvent ["exit"]

[amrbashir]

this is a niche use-case, but I decided to fix it nonetheless.

I decided to go with a different approach by using a wrapping Atomic instead of a mutex and recycling dropped IDs.

I recommend in your app to stop recreating menu items and find a better way to reuse existing items. Hitting the 64.5k is wild and should never happen.