fix: prevent menus from being created after windows

CHANGELOG.md

@@ -25,6 +25,8 @@ project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 * The semi-unstable `iui::draw` subsystem is again exported to downstream consumers of the `iui` crate.
 * `UI::queue_main` and `UI::on_should_quit` now require passed closures to be `'static`, for soundness
 * All callback registration functions require that their callbacks live at least as long as the `UI` token, for soundness
+* `Menu` methods now returns `Option<>` because menus can't always be created or
+modified
 
 ### Deprecated
 
@@ -44,6 +46,7 @@ compliance.
 * Text no longer uses incorrect newlines per platform.
 * `UI::run_delay` no longer spins on the callback, but actually calls it at the
 appropriate interval
+* Menus can no longer be created or modified after windows, as this causes crashes.
 
 ### Security
 

iui/src/controls/window.rs

@@ -4,17 +4,21 @@ use callback_helpers::{from_void_ptr, to_heap_ptr};
 use controls::Control;
 use std::cell::RefCell;
 use std::ffi::{CStr, CString};
+use std::sync::atomic::{Ordering};
 use std::mem;
 use std::os::raw::{c_int, c_void};
 use std::path::PathBuf;
 use ui::UI;
+use menus::{HAS_FINALIZED_MENUS};
 use ui_sys::{self, uiControl, uiWindow};
 
 thread_local! {
     static WINDOWS: RefCell<Vec<Window>> = RefCell::new(Vec::new())
 }
 
 /// A `Window` can either have a menubar or not; this enum represents that decision.\
+///
+/// Once one window is created, no changes to menus can be made.
 #[derive(Clone, Copy, Debug)]
 pub enum WindowType {
     HasMenubar,
@@ -46,6 +50,7 @@ impl Window {
             ));
 
             WINDOWS.with(|windows| windows.borrow_mut().push(window.clone()));
+            HAS_FINALIZED_MENUS.store(true, Ordering::SeqCst);
 
             window
         };

iui/src/lib.rs

@@ -47,3 +47,4 @@ pub mod prelude {
     pub use controls::{Window, WindowType};
     pub use ui::UI;
 }
+

iui/src/menus.rs

@@ -4,18 +4,25 @@ use callback_helpers::{from_void_ptr, to_heap_ptr};
 use controls::Window;
 use std::ffi::CString;
 use std::os::raw::{c_int, c_void};
+use std::sync::atomic::{AtomicBool, Ordering};
 use ui_sys::{self, uiMenu, uiMenuItem, uiWindow};
 use UI;
 
-/// A `MenuItem` represents an item that is shown in a `Menu`. Note that, unlike many controls,
+pub static HAS_FINALIZED_MENUS: AtomicBool = AtomicBool::new(false);
+
+/// A `MenuItem` represents an item that is shown in a `Menu`.
+/// Note that, unlike many controls,
 /// the text on `MenuItem`s cannot be changed after creation.
 #[derive(Clone)]
 pub struct MenuItem {
     ui_menu_item: *mut uiMenuItem,
 }
 
-/// A `Menu` represents one of the top-level menus at the top of a window. As that bar is unique
-/// per application, creating a new `Menu` shows it on all windows that support displaying menus.
+/// A `Menu` represents one of the top-level menus at the top of a window.
+/// That bar is unique per application, and creating a new `Menu` shows it
+/// on all windows that support displaying menus.
+///
+/// Once windows have been created, no more menus can be created.
 #[derive(Clone)]
 pub struct Menu {
     ui_menu: *mut uiMenu,
@@ -77,33 +84,49 @@ impl MenuItem {
 }
 
 impl Menu {
-    /// Creates a new menu with the given name to be displayed in the menubar at the top of the window.
-    pub fn new(_ctx: &UI, name: &str) -> Menu {
-        unsafe {
-            let c_string = CString::new(name.as_bytes().to_vec()).unwrap();
-            Menu {
-                ui_menu: ui_sys::uiNewMenu(c_string.as_ptr()),
+    /// Creates a new menu with the given name to be displayed in the menubar
+    /// at the top of all windows with a menubar.
+    ///
+    /// This is possible only if menus have not been finalized.
+    pub fn new(_ctx: &UI, name: &str) -> Option<Menu> {
+        if HAS_FINALIZED_MENUS.load(Ordering::SeqCst) { None }
+        else {
+            unsafe {
+                let c_string = CString::new(name.as_bytes().to_vec()).unwrap();
+                Some(Menu {
+                    ui_menu: ui_sys::uiNewMenu(c_string.as_ptr()),
+                })
             }
         }
     }
 
     /// Adds a new item with the given name to the menu.
-    pub fn append_item(&self, name: &str) -> MenuItem {
+    ///
+    /// This is possible only if menus have not been finalized.
+    pub fn append_item(&self, name: &str) -> Option<MenuItem> {
+        if HAS_FINALIZED_MENUS.load(Ordering::SeqCst) { None }
+        else {
         unsafe {
             let c_string = CString::new(name.as_bytes().to_vec()).unwrap();
-            MenuItem {
+            Some(MenuItem {
                 ui_menu_item: ui_sys::uiMenuAppendItem(self.ui_menu, c_string.as_ptr()),
-            }
+            })
+        }
         }
     }
 
     /// Adds a new togglable (checkbox) item with the given name to the menu.
-    pub fn append_check_item(&self, name: &str) -> MenuItem {
+    ///
+    /// This is possible only if menus have not been finalized.
+    pub fn append_check_item(&self, name: &str) -> Option<MenuItem> {
+        if HAS_FINALIZED_MENUS.load(Ordering::SeqCst) { None }
+        else {
         unsafe {
             let c_string = CString::new(name.as_bytes().to_vec()).unwrap();
-            MenuItem {
+            Some(MenuItem {
                 ui_menu_item: ui_sys::uiMenuAppendCheckItem(self.ui_menu, c_string.as_ptr()),
-            }
+            })
+        }
         }
     }
 
@@ -112,3 +135,15 @@ impl Menu {
         unsafe { ui_sys::uiMenuAppendSeparator(self.ui_menu) }
     }
 }
+
+#[test]
+fn cannot_change_menus_late() {
+    use crate::prelude::*;
+    let ui = UI::init().expect("failed to init");
+    let mut menu = Menu::new(&ui, "menu").unwrap();
+    assert!(menu.append_item("test item").is_some());
+    let win = Window::new(&ui, "Test App", 200, 200, WindowType::HasMenubar);
+    assert!(Menu::new(&ui, "menu2").is_none());
+    assert!(menu.append_item("test item 2").is_none());
+}
+