bug#63495: 28.2; menu crashes on macos

[Daniel Martín]

Eshel Yaron via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs@gnu.org> writes:

>>> When using the menu - I can call up and see the menu, but if I try to
>>> select a menu item this always results in emacs crashing.
>>
>> I can't reproduce this bug on Emacs 29.  Could you try to reproduce it
>> on this version?  I remember fixing some problems with the macOS menu
>> bar recently, so perhaps this bug is already fixed.
>
> FWIW this seems very similar to Bug#62402.  At least over here (macOS
> 13.4, Emacs master), Emacs still reliably crashes when selecting an item
> in a popup menu.

Thanks, I can reproduce the issue with the recipe in Bug#62402.  I see
that this is a regression in Emacs 28 and Emacs 29.  I can't reproduce
the bug in Emacs 27.

A bisect shows that this is the first commit that introduced the bug:

commit c9b37634b131f3617314bd5a38090e96d0b465cf
Author: Alan Third <alan@idiocy.org>
Date:   Wed Dec 23 20:12:02 2020 +0000

    Remove NS menu synthesized events (bug#44333)
    
    Remove the frame tracking stuff as it's not used for anything, and
    move the update tracking into the EmacsMenu class.
    
    * src/nsmenu.m (ns_update_menubar): Copy the parsing code from xmenu.c
    and rework the NS specific code around to update the existing menus
    instead of rebuilding them completely.
    (ns_activate_menubar):
    ([EmacsMenu trackingNotification:]):
    ([EmacsMenu menuWillOpen:]):
    ([EmacsMenu menuDidClose:]): Remove unused functions.
    ([EmacsMenu menuNeedsUpdate:]): Remove menu tracking code and add code
    to check whether an update is required.
    ([EmacsMenu fillWithWidgetValue:]):
    ([EmacsMenu addSubmenuWithTitle:]):
    ([EmacsMenu initWithTitle:]): Remove references to frame.
    ([EmacsMenu setFrame:]): Remove method.
    ([EmacsMenu clear]): Rename to removeAllItems.
    ([EmacsMenu removeAllItems]): Use built-in removeAllItems, if
    available.
    (syms_of_nsmenu): Remove tracking code.
    * src/nsterm.m (ns_check_menu_open):
    (ns_check_pending_open_menu):
    (ns_create_terminal): Remove unused functions.
    (ns_term_init): Get rid of menu tracking.
    * src/nsterm.h (EmacsMenu): Remove frame, add needsUpdate and update
    method definitions.

One notable thing in that patch is that, in menuNeedsUpdate:,
ns_update_menubar is now called, if needed, in the Cocoa and the GNUstep
build.  If I put the call to ns_update_menubar inside the #ifdef
NS_IMPL_GNUSTEP (so that it's not run in the Cocoa build), I can't
reproduce the crash anymore.  But I don't know if there are any side
effects or what motivated the change in the first place.

I've copied Alan, just in case he has any comments about how we can fix
this issue.