erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.39k stars 2.96k forks source link

wx hangs when quit from Dock on macOS #5603

Open leoliu opened 2 years ago

leoliu commented 2 years ago

Describe the bug wx doesn't handle some Quit events on macOS properly and can hang forever.

To Reproduce

  1. start Erlang on macOS and eval wx:new(), or just start observer like so observer:start()
  2. when the Erlang icon appears in the Dock, right-click to get the context menu and then click Quit
  3. wx hangs

Expected behavior should process such events and do the right thing.

Affected versions Erlang/OTP 24.2, wxwidgets 3.1.5 on macOS Big Sur 11.6.2

Additional context Quitting from the Dock on macOS seems problematic for all apps based on wx (OTP). This might be related to some upstream change as shown in https://forums.wxwidgets.org/viewtopic.php?t=37456

While investigating this issue the beam segment faulted multiple times, a few of them happened when tracing is enabled on a wx_object process. I will report the bug with more details later. Here is an example:

....
Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY

Termination Signal:    Segmentation fault: 11
Termination Reason:    Namespace SIGNAL, Code 0xb
Terminating Process:   exc handler [34643]

VM Regions Near 0:
--> 
    __TEXT                      10e088000-10e3b4000    [ 3248K] r-x/r-x SM=COW  /usr/local/Cellar/erlang/24.2/lib/erlang/erts-12.2/bin/beam.smp

Thread 0:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib          0x00007fff206d72ba mach_msg_trap + 10
1   libsystem_kernel.dylib          0x00007fff206d762c mach_msg + 60
2   com.apple.SkyLight              0x00007fff2503c42b SLSCopyWindowGroup + 298
3   com.apple.AppKit                0x00007fff2325a11f _NSCGSWindowOrderingPropertiesGetOrderingGroupForWindow + 145
4   com.apple.AppKit                0x00007fff23259ff7 -[_NSCGSWindowOrderingProperties removeWindowFromOrderingGroup:] + 45
5   com.apple.AppKit                0x00007fff23259f7e -[_NSCGSWindowOrdering removeWindowFromOrderingGroup:] + 54
6   com.apple.AppKit                0x00007fff2329cbfd -[NSWindow _termWindowIfOwner] + 208
7   com.apple.AppKit                0x00007fff2329b3c1 -[NSWindow dealloc] + 1257
8   libobjc.A.dylib                 0x00007fff205ca20f AutoreleasePoolPage::releaseUntil(objc_object**) + 167
9   libobjc.A.dylib                 0x00007fff205ace30 objc_autoreleasePoolPop + 161
10  com.apple.CoreFoundation        0x00007fff207c54b6 _CFAutoreleasePoolPop + 22
11  com.apple.Foundation            0x00007fff21552123 -[NSAutoreleasePool release] + 131
12  libwx_osx_cocoau_core-3.1.dylib 0x0000000152999f8d wxMacAutoreleasePool::~wxMacAutoreleasePool() + 13
13  libwx_osx_cocoau_core-3.1.dylib 0x000000015297f945 wxApp::~wxApp() + 51
14  wxe_driver.so                   0x00000001524c396e WxeApp::~WxeApp() + 88
....
wojtekmach commented 2 years ago

This is tricky, see these code snippets:

It appears that as long as the Erlang VM that started wx runs, the "app" keeps running: it is in the Dock, can be switched to with cmd+tab, etc.

leoliu commented 2 years ago

I think the linked code snippets might be handling quitting from the Apple Menu only including keyboard shortcuts Cmd + Q.

Quitting from the Dock seems like another beast. When it happens cmd+tab will not bring back the apple menu, nor can you start another GUI window.

I was able to trace a wx_object process while such quitting happened and two things happened. The process received close_window event then immediately followed by a {'_wxe_destroy_',_Me} message which is taken care of by the behaviour module wx_object by calling terminate/2. See https://github.com/erlang/otp/blob/eef2e7066ecdf9de5dc7fd81dc5043d9a9757efa/lib/wx/src/wx_object.erl#L458

If terminate/2 implementation has a call to destroy which is almost always there is a deadlock. This might be another bug.

wojtekmach commented 2 years ago

Right. So eg observer correctly handles cmd+q, it quits, kills the observer frame, but the Erlang VM keeps running and the app is still in the dock. You can see that in the way menu bar is changing, when observer was running we had "Erlang", "File", "View", "Nodes", etc menus. When observer quit, we have just "Erlang" & "Window" menus. Not saying there's not a bug, btw, just trying to add additional context!

leoliu commented 2 years ago

Yes quitting from the Apple Menu is handled as command_menu_selected with id ?wxID_EXIT. But quitting by right-click on the AppIcon on the Dock is what's nasty.

dgud commented 2 years ago

My private mac have died so I have problems debugging on mac, so I need some help with this.

Mac doesn't really like mixing console command and gui and erlang tries exactly that, so it requires some hacks, and in erlang can you bring up several gui applications from the same erlang shell, e.g. observer and debugger, no one have a gui-api that can do that or expect that (at least wxWidgets doesn't). They also expect that when you close your gui you exit your application, which we don't always want to do. Mac API also expects that the gui thread is the main thread, (what is a main-thread anyway), which means that wx have steal the first thread from erlang to have any gui at all and return that when exiting.

But if you can help out with improvements in this area it would be great.

wojtekmach commented 2 years ago

@dgud @leoliu I did some digging and found how wx handles the "quit" app event: (kAEQuitApplication)

The wxTheApp->OSXOnWillTerminate() method can be overloaded.

Here's one possible implementation, one where we send a {quit_app, []} message. This is the same type of thing as {open_url, Url} etc we already have for Mac. You need to call wx:subscribe_events() to receive it. Here's a patch:

commit 86284130513818aa6923ee0fbbef621afddc298f
Author: Wojtek Mach <wojtek@wojtekmach.pl>
Date:   Sat Apr 16 23:21:29 2022 +0200

    wip

diff --git a/lib/wx/c_src/wxe_impl.cpp b/lib/wx/c_src/wxe_impl.cpp
index 67b319b036..f49d3e91c5 100644
--- a/lib/wx/c_src/wxe_impl.cpp
+++ b/lib/wx/c_src/wxe_impl.cpp
@@ -224,6 +224,11 @@ void WxeApp::MacReopenApp() {
   wxString empty;
   send_msg("reopen_app", &empty);
 }
+
+void WxeApp::OSXOnWillTerminate() {
+  wxString empty;
+  send_msg("quit_app", &empty);
+}
 #endif

 void WxeApp::shutdown(wxeMetaCommand& Ecmd) {
@@ -233,7 +238,8 @@ void WxeApp::shutdown(wxeMetaCommand& Ecmd) {
 }

 void WxeApp::dummy_close(wxEvent& Ev) {
-  // fprintf(stderr, "Dummy Close invoked\r\n");
+  wxString empty;
+  send_msg("quit_app", &empty);
   // wxMac really wants a top level window which command-q quits if there are no
   // windows open, and this will kill the erlang, override default handling
 }
diff --git a/lib/wx/c_src/wxe_impl.h b/lib/wx/c_src/wxe_impl.h
index 16487f3c0d..9210683f16 100644
--- a/lib/wx/c_src/wxe_impl.h
+++ b/lib/wx/c_src/wxe_impl.h
@@ -64,6 +64,7 @@ public:
   virtual void MacOpenURL(const wxString &url);
   virtual void MacNewFile();
   virtual void MacReopenApp();
+  virtual void OSXOnWillTerminate();
 #endif

   void init_consts(wxeMetaCommand& event);
diff --git a/lib/wx/src/wxe_master.erl b/lib/wx/src/wxe_master.erl
index 5724457ae7..c899e9f707 100644
--- a/lib/wx/src/wxe_master.erl
+++ b/lib/wx/src/wxe_master.erl
@@ -178,7 +178,7 @@ handle_info({wxe_driver, debug, Msg}, State) ->
     {noreply, State};
 handle_info({wxe_driver, Cmd, File}, State = #state{subscribers=Subs, msgs=Msgs}) 
   when Cmd =:= open_file; Cmd =:= new_file; Cmd =:= print_file; 
-       Cmd =:= open_url; Cmd =:= reopen_app ->
+       Cmd =:= open_url; Cmd =:= reopen_app; Cmd =:= quit_app ->
     lists:foreach(fun(Pid) -> Pid ! {Cmd, File} end, Subs),
     {noreply, State#state{msgs=[{Cmd, File}|Msgs]}};
 handle_info({'DOWN', _Ref, process, Pid, _Info}, State) ->

(https://github.com/wojtekmach/otp/commit/86284130513818aa6923ee0fbbef621afddc298f)

And here's an example shell session:

$ erl
wx:new(), wx:subscribe_events(), receive {quit_app, []} -> halt() end.

I even thrown in a default implementation of the "Quit" menu item that sends the same message. While we can do whatever we want in the menu item handler, Apple really wants us to quit the app in the app quit handler (we get a beachball otherwise), thus the only sensible thing to do there is to init:stop() or init:halt() however we of course have an opportunity to do some cleanup logic.

Please let me know if this seems worth pursuing!

leoliu commented 2 years ago

Thank you so much for looking into this issue. I think wx:subscribe_events/0 has potential ;)

I was investigating this issue in Jan however my C++ skill probably made it much harder. But I think this info is relevant (taken from https://docs.wxwidgets.org/latest/classwx_close_event.html):

The EVT_END_SESSION event is slightly different as it is sent by the system when the user session is ending (e.g. because of log out or shutdown) and so all windows are being forcefully closed. At least under MSW, after the handler for this event is executed the program is simply killed by the system. Because of this, the default handler for this event provided by wxWidgets calls all the usual cleanup code (including wxApp::OnExit()) so that it could still be executed and exit()s the process itself, without waiting for being killed. If this behaviour is for some reason undesirable, make sure that you define a handler for this event in your wxApp-derived class and do not call event.Skip() in it (but be aware that the system will still kill your application).

Also this bug may also happen on other platforms. On windows for example, let's say your wx app is rendering a website using webview with edge backend. Go and uninstall the webview2 SDK.

Another issue is before quitting, close_window and {'_wxe_destroy_', Self} are sent by the wxe_driver. The latter invokes Module:terminate/2 callback. If your code invokes any functions (such as destroy/1) that need to speak to the wxe_driver it feels like it is deadlocking.