1804978 - Crash in [@ -[MOZMenuOpeningCoordinator _runMenu]] with !mRunMenuIsOnTheStack

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/47c42691-017a-49ea-bf48-e91d60221209

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(!mRunMenuIsOnTheStack)

Top 10 frames of crashing thread:

0  XUL  -[MOZMenuOpeningCoordinator _runMenu]  widget/cocoa/MOZMenuOpeningCoordinator.mm:92
1  Foundation  __NSFireDelayedPerform  
2  CoreFoundation  __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__  
3  CoreFoundation  __CFRunLoopDoTimer  
4  CoreFoundation  __CFRunLoopDoTimers  
5  CoreFoundation  __CFRunLoopRun  
6  CoreFoundation  CFRunLoopRunSpecific  
7  Foundation  -[NSRunLoop runMode:beforeDate:]  
8  Foundation  -[NSRunLoop runUntilDate:]  
9  AppKit  NSCoreDragTrackingProc  

This release assert was added in bug 1707598. The volume is low, but perhaps the stacks are interesting.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:spohl, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Stephen A Pohl [:spohl] (OOO until 3/20)]

It looks like _runMenu is already on the stack here. Markus, could you take a look to see what might be going on here?

Comment 3

[Markus Stange [:mstange]]

Hmm, interesting: We only enqueue a delayed perform of _runMenu if mRunMenuIsOnTheStack is false:

https://searchfox.org/mozilla-central/rev/75da1dd5d4b9b991f919a41594194eab93cdef62/widget/cocoa/MOZMenuOpeningCoordinator.mm#83-86

if (!mRunMenuIsOnTheStack) {
  // Call _runMenu from the event loop, so that it doesn't block this call.
  [self performSelector:@selector(_runMenu) withObject:nil afterDelay:0.0];
}

But then mRunMenuIsOnTheStack is true by the time the delayed _runMenu perform is executed.

So it seems that, between the time at which this code runs, and the time at which the delayed perform is executed, something else causes us to enter _runMenu. But I don't understand how that's possible - if another delayed perform for _runMenu was already pending, then we should have crashed in the !mPendingOpening assert further up in the same function.

Comment 4

[Stephen A Pohl [:spohl] (OOO until 3/20)]

Attachment: Bug 1804978: Prevent release assert crashes due to a race when opening context menus on macOS. r=mstange!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/149e240e2c11
Prevent release assert crashes due to a race when opening context menus on macOS. r=mstange

Comment 6

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/149e240e2c11

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:spohl, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Comment 8

[Stephen A Pohl [:spohl] (OOO until 3/20)]

I don't believe that the crash rate here warrants an uplift and even thought he patch seems straightforward, it could benefit from more bake time.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

What are your thoughts about taking this on ESR?

Comment 10

[Stephen A Pohl [:spohl] (OOO until 3/20)]

Comment on attachment 9373046 [details]
Bug 1804978: Prevent release assert crashes due to a race when opening context menus on macOS. r=mstange!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This prevents a crash due to a possible race when opening context menus on macOS.
• User impact if declined: Possibility of crashes when opening context menus on macOS.
• Fix Landed on Version: 123
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch is minimal and well understood. The patch adds a native API call that cancels the opening of a context menu when the user is attempting to open a different/new context menu.

Comment 11

[Dianna Smith [:diannaS]]

Comment on attachment 9373046 [details]
Bug 1804978: Prevent release assert crashes due to a race when opening context menus on macOS. r=mstange!

Approved for 115.9esr

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/aab792a24e14