[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#63495: 28.2; menu crashes on macos
From: |
Daniel Martín |
Subject: |
bug#63495: 28.2; menu crashes on macos |
Date: |
Thu, 13 Jul 2023 10:47:35 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (darwin) |
Eshel Yaron via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs@gnu.org> writes:
> Alan Third <alan@idiocy.org> writes:
>
>> Can you please try this:
>>
>> modified src/nsmenu.m
>> @@ -746,6 +746,8 @@ - (Lisp_Object)runMenuAt: (NSPoint)p forFrame: (struct
>> frame *)f
>> NSEvent *e, *event;
>> long retVal;
>>
>> + needsUpdate = NO;
>> +
>> /* p = [view convertPoint:p fromView: nil]; */
>> p.y = NSHeight ([view frame]) - p.y;
>> e = [[view window] currentEvent];
>>
>> At a guess, when the menu opens the first thing AppKit does is check if
>> it needs updated, and since a new menu starts with needsUpdate=YES, it
>> goes ahead and tries to do it, which overwrites some important
>> variables from the original "build" of the menu.
>>
>> The context menu is built and then displayed, as opposed to the
>> top-menu that is partially built, then when it's to be displayed is
>> updated and filled in.
>
> I tried it, unfortunately, that doesn't seem to solve the issue. Emacs
> still crashes with a similar backtrace:
>
> * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS
> (code=1, address=0x3)
> frame #0: 0x00000001000af1fd emacs`AREF(array=0x0000000000000000, idx=0)
> at lisp.h:1941:10
> 1938 AREF (Lisp_Object array, ptrdiff_t idx)
> 1939 {
> 1940 eassert (0 <= idx && idx < gc_asize (array));
> -> 1941 return XVECTOR (array)->contents[idx];
> 1942 }
> 1943
> 1944 INLINE Lisp_Object *
> Target 0: (emacs) stopped.
> (lldb) bt
> * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS
> (code=1, address=0x3)
> * frame #0: 0x00000001000af1fd emacs`AREF(array=0x0000000000000000, idx=0)
> at lisp.h:1941:10
> frame #1: 0x00000001000b079e
> emacs`find_and_return_menu_selection(f=0x00000001030b0c30, keymaps=true,
> client_data=0x0000000110277e60) at menu.c:985:11
> frame #2: 0x000000010037cc1a emacs`-[EmacsMenu
> runMenuAt:forFrame:keymaps:](self=0x000060000179c9c0,
> _cmd="runMenuAt:forFrame:keymaps:", p=(x = 2, y = 411), f=0x00000001030b0c30,
> keymaps=true) at nsmenu.m:769:9
> frame #3: 0x0000000100380ac0 emacs`ns_menu_show(f=0x00000001030b0c30,
> x=2, y=97, menuflags=1, title=0x0000000102416284, error=0x00007ff7bfefcef0)
> at nsmenu.m:1069:9
> frame #4: 0x00000001000b23f7
> emacs`x_popup_menu_1(position=0x0000000118583f83, menu=0x0000000118587313) at
> menu.c:1410:17
> frame #5: 0x00000001000b27d2
> emacs`Fx_popup_menu(position=0x0000000118583f83, menu=0x0000000118587313) at
> menu.c:1474:10
> frame #6: 0x000000010024e7d7 emacs`funcall_subr(subr=0x0000000100403c28,
> numargs=2, args=0x0000000118028188) at eval.c:3049:15
One fundamental problem I see with the current logic (or maybe I'm
misunderstanding something) is that, menuNeedsUpdate: is called for both
the menubar and for context menus. However, the ns_update_menubar
routine does not even use the NSMenu that AppKit passed to
menuNeedsUpdate:, it always does this:
id menu = [NSApp mainMenu];
This means that the menu variable will always be the menubar. The code
in ns_update_menubar is only prepared to handle menubar updates, but as
this function updates menu_items, a data structure that is used later by
the context menu in find_and_return_menu_selection, Emacs crashes
because of inconsistencies.
Is there any reason we need to do something for context menus in
menuNeedsUpdate:? Alan said that context menus are completely built in
advance (as opposed to the menubar, which is partially built), so I
propose the following patch (which does seem to fix the crash for me and
doesn't cause regressions with the menubar):
diff --git a/src/nsmenu.m b/src/nsmenu.m
index 2c1f575bdf2..ca367d1abd1 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -477,6 +477,16 @@ - (instancetype)initWithTitle: (NSString *)title
call to ns_update_menubar. */
- (void)menuNeedsUpdate: (NSMenu *)menu
{
+
+ /* The context menu is built and then displayed, as opposed to the
+ top-menu, which is partially built and then updated and filled in
+ when it's time to display it. Therefore, we don't call
+ ns_update_menubar if a context menu is active. */
+ if (context_menu_value != 0)
+ {
+ return;
+ }
+
#ifdef NS_IMPL_GNUSTEP
static int inside = 0;
#endif
- bug#63495: 28.2; menu crashes on macos, Daniel Martín, 2023/07/08
- bug#63495: 28.2; menu crashes on macos, Eshel Yaron, 2023/07/09
- bug#63495: 28.2; menu crashes on macos, Daniel Martín, 2023/07/10
- bug#63495: 28.2; menu crashes on macos, Alan Third, 2023/07/10
- bug#63495: 28.2; menu crashes on macos, Eshel Yaron, 2023/07/11
- bug#63495: 28.2; menu crashes on macos, Alan Third, 2023/07/12
- bug#63495: 28.2; menu crashes on macos, Eshel Yaron, 2023/07/12
- bug#63495: 28.2; menu crashes on macos, Alan Third, 2023/07/12
- bug#63495: 28.2; menu crashes on macos, Eshel Yaron, 2023/07/13
- bug#63495: 28.2; menu crashes on macos,
Daniel Martín <=
- bug#63495: 28.2; menu crashes on macos, Alan Third, 2023/07/13
- bug#63495: 28.2; menu crashes on macos, Alan Third, 2023/07/25
- bug#63495: 28.2; menu crashes on macos, Eli Zaretskii, 2023/07/25