macOS: BrowserWindow.destroy() can cause segfaults

[sa-MatteoHausner]

Preflight Checklist

• I have read the Contributing Guidelines for this project.
• I agree to follow the Code of Conduct that this project adheres to.
• I have searched the issue tracker for an issue that matches the one I want to file, without success.

Issue Details

• Electron Version:
  • 4.1.1
• Operating System:
  • macOS 10.14.3
• Last Known Working Electron version:
  • Unknown

Expected Behavior

Creating a modal BrowserWindow and closing it via destroy() on macOS, followed by creating a second modal BrowserWindow and closing it via close() should not produce any visual artifcats, nor leave the application in a inoperable state, nor lead to segmentation faults.

Actual Behavior

Creating a modal BrowserWindow and closing it via destroy(), followed by creating a second modal BrowserWindow and closing it via close() summons parts (visual artifacts) of the previously destroyed first BrowserWindow (see our test-case and attached screenshot).
Afterwards the application does not react to any input anymore and must be terminated forcefully.
On MacBookPros with a touchbar, in many cases, the application crashes with a segmentation fault or illegal hardware instruction:

Application Specific Information:
Crashing on exception: -[__NSTaggedDate makeTouchBar]: unrecognized selector sent to instance 0x800007fc55bf8e7d

Application Specific Backtrace 1:
0   CoreFoundation                      0x00007fff2bc3aded __exceptionPreprocess + 256
1   libobjc.A.dylib                     0x00007fff57d02720 objc_exception_throw + 48
2   CoreFoundation                      0x00007fff2bcb8195 -[NSObject(NSObject) __retain_OA] + 0
3   CoreFoundation                      0x00007fff2bbdca60 ___forwarding___ + 1486
4   CoreFoundation                      0x00007fff2bbdc408 _CF_forwarding_prep_0 + 120
5   Electron Framework                  0x000000010d421c38 _ZN4atom19AtomNativeWidgetMac14CreateNSWindowERKN5views6Widget10InitParamsE + 568
6   AppKit                              0x00007fff291d4ba8 -[NSResponder(NSTouchBarProvider) touchBar] + 56
7   AppKit                              0x00007fff297fed5e NSTouchBarFinderTouchBarsForProviders + 279
8   AppKit                              0x00007fff2980011f _NSTouchBarFinderUpdate + 2214
9   AppKit                              0x00007fff297ff857 ___NSTouchBarFinderSetNeedsUpdateOnMain_block_invoke + 48
10  CoreFoundation                      0x00007fff2bbe18ed __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
11  CoreFoundation                      0x00007fff2bbe1822 __CFRunLoopDoObservers + 452
12  CoreFoundation                      0x00007fff2bb82ca0 CFRunLoopRunSpecific + 523
13  HIToolbox                           0x00007fff2ae19ab5 RunCurrentEventLoopInMode + 293
14  HIToolbox                           0x00007fff2ae197eb ReceiveNextEventCommon + 618
15  HIToolbox                           0x00007fff2ae19568 _BlockUntilNextEventMatchingListInModeWithFilter + 64
16  AppKit                              0x00007fff290d4363 _DPSNextEvent + 997
17  AppKit                              0x00007fff290d3102 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1362
18  AppKit                              0x00007fff290cd165 -[NSApplication run] + 699
19  Electron Framework                  0x000000010d61d78c _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 1306348
20  Electron Framework                  0x000000010d61c46e _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 1301454
21  Electron Framework                  0x000000010d5ab3a5 _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 838405
22  Electron Framework                  0x000000010da72cf0 _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 5850192
23  Electron Framework                  0x000000010da72b20 _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 5849728
24  Electron Framework                  0x000000010da9dfd2 _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 6027058
25  Electron Framework                  0x000000010da6ee29 _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 5834121
26  Electron Framework                  0x000000010d7647b9 _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 2645785
27  Electron Framework                  0x000000010f40dff2 _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 32700242
28  Electron Framework                  0x000000010d7633d4 _ZN8crashpad8internal20ScopedDIRCloseTraits4FreeEP3DIR + 2640692
29  Electron Framework                  0x000000010d349f04 AtomMain + 68
30  Electron                            0x000000010a4ddf16 main + 38
31  libdyld.dylib                       0x00007fff58dd0ed9 start + 1
32  ???                                 0x0000000000000002 0x0 + 2

To Reproduce

To reproduce run our test-case as follows:

$ git clone https://github.com/secadm/electron-bugs.git -b mac-window-destroy
$ npm install
$ npm start

Screenshots

The first modal BrowserWindow:

The second modal BrowserWindow (the first on has been destroyed):

The second modal BrowserWindow has been closed and the silhouette of the first one has reappeared:

Additional Information

• The issue seems to be present at least since Electron 3.0.0.
• The issue is only present on macOS and not on Windows.
• The issue only appears if the first BrowserWindow gets closed via destroy() and the second one via close(). If the second modal BrowserWindow also gets closed via destroy() the issue does not appear. Also if both BrowserWindows get closed via close() everything seems to be okay.

[codebytere]

cc @MarshallOfSound for potentially TouchBar-related crash

[WesUnwin]

Currently experiencing this issue, am on Electron 15.0.0, it indeed results in the entire app crashing. I believe it is the same issue based on the Mac OS crash report having an identical error + call stack.

[nornagon]

I hit this issue while running the test suite today:

Received signal 11 <unknown> 000000000000
0   Electron Framework                  0x00000001264e8559 base::debug::CollectStackTrace(void**, unsigned long) + 9
1   Electron Framework                  0x0000000126408d13 base::debug::StackTrace::StackTrace() + 19
2   Electron Framework                  0x00000001264e84b1 base::debug::(anonymous namespace)::StackDumpSignalHandler(int, __siginfo*, void*) + 1393
3   libsystem_platform.dylib            0x00007ff8058fde2d _sigtramp + 29
4   libsystem_malloc.dylib              0x00007ff805706622 nanov2_calloc + 126
5   AppKit                              0x00007ff808a4fd13 NSTouchBarFinderTouchBarsForProviders + 438
6   AppKit                              0x00007ff808a5100c ___NSTouchBarFinderSetNeedsUpdateOnMain_block_invoke_2 + 2361
7   AppKit                              0x00007ff8084a850c NSDisplayCycleObserverInvoke + 155
8   AppKit                              0x00007ff8084a8097 NSDisplayCycleFlush + 952
9   QuartzCore                          0x00007ff80ca2d384 CA::Transaction::run_commit_handlers(CATransactionPhase) + 98
10  QuartzCore                          0x00007ff80ca2c102 CA::Transaction::commit() + 376
11  AppKit                              0x00007ff8085497d2 __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke + 285
12  AppKit                              0x00007ff808c920d4 ___NSRunLoopObserverCreateWithHandler_block_invoke + 41
13  CoreFoundation                      0x00007ff8059aecb7 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
14  CoreFoundation                      0x00007ff8059aeb54 __CFRunLoopDoObservers + 543
15  CoreFoundation                      0x00007ff8059adfe7 __CFRunLoopRun + 841
16  CoreFoundation                      0x00007ff8059ad5dd CFRunLoopRunSpecific + 563
17  HIToolbox                           0x00007ff80e5ea4f1 RunCurrentEventLoopInMode + 292
18  HIToolbox                           0x00007ff80e5ea247 ReceiveNextEventCommon + 587
19  HIToolbox                           0x00007ff80e5e9fe5 _BlockUntilNextEventMatchingListInModeWithFilter + 70
20  AppKit                              0x00007ff8083dcd88 _DPSNextEvent + 886
21  AppKit                              0x00007ff8083db3f4 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1411
22  AppKit                              0x00007ff8083cd919 -[NSApplication run] + 586
23  Electron Framework                  0x00000001264feb2c base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 348
24  Electron Framework                  0x00000001264fd4c2 base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 130
25  Electron Framework                  0x00000001264a5c79 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) + 681
26  Electron Framework                  0x0000000126457f26 base::RunLoop::Run(base::Location const&) + 726
27  Electron Framework                  0x0000000125066763 content::BrowserMainLoop::RunMainMessageLoop() + 243
28  Electron Framework                  0x0000000125068342 content::BrowserMainRunnerImpl::Run() + 82
29  Electron Framework                  0x0000000125063a5e content::BrowserMain(content::MainFunctionParams) + 270
30  Electron Framework                  0x0000000121e807e2 content::RunBrowserProcessMain(content::MainFunctionParams, content::ContentMainDelegate*) + 258
31  Electron Framework                  0x0000000121e81d62 content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams, bool) + 1410
32  Electron Framework                  0x0000000121e81760 content::ContentMainRunnerImpl::Run() + 736
33  Electron Framework                  0x0000000121e80103 content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) + 2723
34  Electron Framework                  0x0000000121e8021c content::ContentMain(content::ContentMainParams) + 92
35  Electron Framework                  0x0000000121a4184d ElectronMain + 157
36  dyld                                0x000000010b6aa4fe start + 462
[end of stack trace]

[nornagon]

Did some debugging, it looks like in some cases it is possible for an ElectronNSWindow to outlive the NativeWindowMac to which it holds a reference in shell_. Then, AppKit calls -[NSResponder makeTouchBar] which we implement here. We reach into shell_, but shell_ has since been deallocated, making a use-after-free bug.

[nornagon]

I think I hit a different stack trace possibly with the same underlying cause in the CI today:

0   Electron Framework                  0x000000011924a599 base::debug::CollectStackTrace(void**, unsigned long) + 9
1   Electron Framework                  0x000000011916ad53 base::debug::StackTrace::StackTrace() + 19
2   Electron Framework                  0x000000011924a4f1 base::debug::(anonymous namespace)::StackDumpSignalHandler(int, __siginfo*, void*) + 1393
3   libsystem_platform.dylib            0x00007fff6a7935fd _sigtramp + 29
4   ???                                 0x0000000300000004 0x0 + 12884901892
5   QuickLookUI                         0x00007fff3c111096 __40-[QLPreviewPanel closingCompletionBlock]_block_invoke + 45
6   QuickLookUI                         0x00007fff3c0fbb54 -[QLWindowEffect done] + 34
7   QuickLookUI                         0x00007fff3c0fba04 -[QLAnimationWindowEffect done] + 101
8   QuickLookUI                         0x00007fff3c181b60 __26-[QLFadeWindowEffect done]_block_invoke + 58
9   QuickLookUI                         0x00007fff3c0fbd70 -[QLFadeWindowEffect done] + 65
10  Foundation                          0x00007fff32e0a0dd __NSThreadPerformPerform + 204
11  CoreFoundation                      0x00007fff30738de2 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
12  CoreFoundation                      0x00007fff30738d81 __CFRunLoopDoSource0 + 103
13  CoreFoundation                      0x00007fff30738b9b __CFRunLoopDoSources0 + 209
14  CoreFoundation                      0x00007fff307378ca __CFRunLoopRun + 927
15  CoreFoundation                      0x00007fff30736ece CFRunLoopRunSpecific + 462
16  HIToolbox                           0x00007fff2f365abd RunCurrentEventLoopInMode + 292
17  HIToolbox                           0x00007fff2f3657d5 ReceiveNextEventCommon + 584
18  HIToolbox                           0x00007fff2f365579 _BlockUntilNextEventMatchingListInModeWithFilter + 64
19  AppKit                              0x00007fff2d9ad829 _DPSNextEvent + 883
20  AppKit                              0x00007fff2d9ac070 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1352
21  AppKit                              0x00007fff2d99dd7e -[NSApplication run] + 658
[...]

CI link: https://app.circleci.com/pipelines/github/electron/electron/51363/workflows/2a8d7ccd-f935-4b9b-b5ee-19b101eed0d4/jobs/1174703

[codebytere]

This issue is caused when [window close] is called on a window currently in the middle of a fullscreen transition. As of now there's not a great way to defer closing during destruction, since the window gets released but i put up a PR to address a similar issue: #34378

[github-actions]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

[github-actions]

This issue has been closed due to inactivity, and will not be monitored. If this is a bug and you can reproduce this issue on a supported version of Electron please open a new issue and include instructions for reproducing the issue.