[bug] `Icon` leaked by `tao::system_tray::SystemTray::set_icon` on Windows

[Lej77]

Describe the bug

On Windows the tao::system_tray::SystemTray::set_icon method leaks the tao::system_tray::Icon argument that is passed to it. Since that Icon is backed by a WinIcon this will be noticed relatively quickly since there is a low limit on the number of handles per process which leads to the not very helpful error Runtime(InvalidIcon(OsError(Os { code: 0, kind: Uncategorized, message: "The operation completed successfully." }))) when setting a tray icon too many times (about 3000 times in my case) using for example Tauri.

Steps To Reproduce

I made a modified version of tao/examples/system_tray.rs that tries setting the tray icon 10 000 times. This eventually causes the program to panic with the following message:

thread 'main' panicked at 'Failed to open icon: OsError(Os { code: 0, kind: Uncategorized, message: "The operation completed successfully." })', src\main.rs:44:10

Code

Create a new cargo project then add crates using: ```bash cargo add image cargo add tao --features tray ``` Then copy the following code into `src/main.rs`: ```rust // Copyright 2014-2021 The winit contributors // Copyright 2021-2023 Tauri Programme within The Commons Conservancy // SPDX-License-Identifier: Apache-2.0 // System tray is supported and availabled only if `tray` feature is enabled. // Platform: Windows, Linux and macOS. #[cfg(any(target_os = "windows", target_os = "linux", target_os = "macos"))] fn main() { use tao::{ event_loop::{EventLoop}, system_tray::SystemTrayBuilder, TrayId, }; let event_loop = EventLoop::new(); let path = concat!(env!("CARGO_MANIFEST_DIR"), "/src/icon.png"); let main_tray_id = TrayId::new("main-tray"); let icon = load_icon(std::path::Path::new(path)); #[cfg(target_os = "windows")] let mut system_tray = SystemTrayBuilder::new(icon, None) .with_id(main_tray_id) .build(&event_loop) .unwrap(); for _ in 0..10_000 { system_tray.set_icon(load_icon(std::path::Path::new(path))); } } #[cfg(any(target_os = "windows", target_os = "linux", target_os = "macos"))] fn load_icon(path: &std::path::Path) -> tao::system_tray::Icon { let (icon_rgba, icon_width, icon_height) = { let image = image::open(path) .expect("Failed to open icon path") .into_rgba8(); let (width, height) = image.dimensions(); let rgba = image.into_raw(); (rgba, width, height) }; tao::system_tray::Icon::from_rgba(icon_rgba, icon_width, icon_height) .expect("Failed to open icon") } // System tray isn't supported on other's platforms. #[cfg(not(any(target_os = "windows", target_os = "linux", target_os = "macos")))] fn main() { println!("This platform doesn't support system_tray."); } ```

Expected behavior

The program should not panic and instead exit successfully.

Screenshots

Platform and Versions: OS: Windows 10 Rustc: 1.70.0 (90c541806 2023-05-31)

Would you want to assign yourself to resolve this bug?

• [ ] Yes
• [ ] No

Additional context

Source of the leak

In tao the Windows specific SystemTray::set_icon method puts an Icon into the "window"'s state, this instance of the Icon(Arc<..>) will no longer be dropped automatically after this point.

https://github.com/tauri-apps/tao/blob/50e69d718e9c71d044ebc3535ca58a992db18547/src/platform_impl/windows/system_tray.rs#L185

The sent message (with a pointer to a boxed Arc) is handled at:

https://github.com/tauri-apps/tao/blob/50e69d718e9c71d044ebc3535ca58a992db18547/src/platform_impl/windows/system_tray.rs#L275

This method calls .clone() on the data even though it was already owned. It should use Box::from_raw instead.

Info I gathered when searching for this bug

The tray animation freezes the app after running for about 1-2h when updating the icon every 500ms. Trying to open a Tauri window using the tray's context menu after that returns the following error: ```text Runtime(InvalidIcon(OsError(Os { code: 0, kind: Uncategorized, message: "The operation completed successfully." }))) ``` ##### Conclusion: resource exhaustion The error can happen much faster if the icon is changed more often. In this case it happened after setting the tray icon a little more than 3000 times. By searching for `Windows CreateIcon The operation completed successfully` on Google I found: - [c# - System.Drawing.Icon constructor throwing "Operation completed successfully" exception - Stack Overflow](https://stackoverflow.com/questions/2356580/system-drawing-icon-constructor-throwing-operation-completed-successfully-exce) Which lead me to Google for `CSharp System.Drawing.Icon The operation completed successfully` where I found: - [c# - System.ComponentModel.Win32Exception: The operation completed successfully - Stack Overflow](https://stackoverflow.com/questions/1209769/system-componentmodel-win32exception-the-operation-completed-successfully) - [c# - this.Icon = ((System.Drawing.Icon)(resources.GetObject("$this.Icon"))); exception on Windows 10? - Stack Overflow](https://stackoverflow.com/questions/70656001/this-icon-system-drawing-iconresources-getobjectthis-icon-exceptio) - ["Win32Exception The operation completed successfully" on IconSourcePropertyChanged · Issue #52 · hardcodet/wpf-notifyicon](https://github.com/hardcodet/wpf-notifyicon/issues/52) In most of those links they talk about not releasing resources. One reply on StackOverflow even mentioned that there is a limit of 10 000 handles per process: - [c# - System.ComponentModel.Win32Exception: The operation completed successfully - Stack Overflow](https://stackoverflow.com/questions/1209769/system-componentmodel-win32exception-the-operation-completed-successfully/6428680#6428680) ##### Follow error type: I think that this error type is only construed at: https://github.com/tauri-apps/tauri/blob/3065c8aea375535763e1532951c4057a426fce80/core/tauri-runtime-wry/src/lib.rs#L463 So the error comes from `WryWindowIcon::from_rgba` which is an alias for `wry::application::window::Icon` which in turn is a re-export of `tao::window::Icon`. The called method is therefore at: https://github.com/tauri-apps/tao/blob/50e69d718e9c71d044ebc3535ca58a992db18547/src/icon.rs#L137 That method delegates to platform specific code which for Windows should be at: https://github.com/tauri-apps/tao/blob/50e69d718e9c71d044ebc3535ca58a992db18547/src/platform_impl/windows/icon.rs#L127 That method can fail at the Windows specific `into_windows_icon` method at: https://github.com/tauri-apps/tao/blob/50e69d718e9c71d044ebc3535ca58a992db18547/src/platform_impl/windows/icon.rs#L25C6-L25C23 Or at the `RgbaIcon::from_rgba` method which is shared between multiple platforms at: https://github.com/tauri-apps/tao/blob/50e69d718e9c71d044ebc3535ca58a992db18547/src/icon.rs#L87 These methods all return a `BadIcon` error of which we caught the `OsError` variant: https://github.com/tauri-apps/tao/blob/50e69d718e9c71d044ebc3535ca58a992db18547/src/icon.rs#L37 `RgbaIcon::from_rgba` doesn't construct an OsError variant so that method doesn't seem like it could be the cause. The Windows specific `into_windows_icon` method does construct the OsError variant so that is likely the cause. The method preforms some unsafe array manipulations and then call Windows `CreateIcon` function. ##### Drill down into tray_handle().set_icon() Likely eventually sends a message of type `TrayMessage::UpdateIcon` which gets handled at: https://github.com/tauri-apps/tauri/blob/dev/core/tauri-runtime-wry/src/lib.rs#L2624 Which calls `TrayIcon::try_from(icon)`, `TrayIcon` is defined in the sibling file: https://github.com/tauri-apps/tauri/blob/dev/core/tauri-runtime-wry/src/system_tray.rs#L77 Where it calls `WryTrayIcon::from_rgba`. `WryTrayIcon` is an alias for `wry::application::system_tray::Icon` which in turn is a re-export of `tao::system_tray::Icon`. That module re-exports it from `tao::icon::Icon`: https://github.com/tauri-apps/tao/blob/dev/src/system_tray.rs#L42

[Lej77]

It is likely that updating the tray's tooltip also leaks. But in that case the leaked data is only a String so it isn't as bad.

Data is passed into the window message here using Box::into_raw:

https://github.com/tauri-apps/tao/blob/50e69d718e9c71d044ebc3535ca58a992db18547/src/platform_impl/windows/system_tray.rs#L217

And later cloned from a reference to that data:

https://github.com/tauri-apps/tao/blob/50e69d718e9c71d044ebc3535ca58a992db18547/src/platform_impl/windows/system_tray.rs#L282

Box::from_raw should probably be used here as well.

[amrbashir]

@Lej77 thanks for the detailed report, would you care to open a PR?

[Lej77]

@amrbashir I have opened #755 with the suggested fix!