Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wx hangs when quit from Dock on macOS #5603

Open
leoliu opened this issue Jan 12, 2022 · 7 comments
Open

wx hangs when quit from Dock on macOS #5603

leoliu opened this issue Jan 12, 2022 · 7 comments
Assignees
Labels
bug Issue is reported as a bug help wanted Issue not worked on by OTP; help wanted from the community priority:low team:PS Assigned to OTP team PS

Comments

@leoliu
Copy link
Contributor

leoliu commented Jan 12, 2022

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
....
@leoliu leoliu added the bug Issue is reported as a bug label Jan 12, 2022
@wojtekmach
Copy link
Contributor

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
Copy link
Contributor Author

leoliu commented Jan 14, 2022

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

{'_wxe_destroy_', _Me} ->

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

@wojtekmach
Copy link
Contributor

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
Copy link
Contributor Author

leoliu commented Jan 14, 2022

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.

@IngelaAndin IngelaAndin added the team:PS Assigned to OTP team PS label Jan 14, 2022
@dgud
Copy link
Contributor

dgud commented Jan 14, 2022

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
Copy link
Contributor

@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) ->

(wojtekmach@8628413)

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
Copy link
Contributor Author

leoliu commented Apr 17, 2022

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.

@IngelaAndin IngelaAndin added the help wanted Issue not worked on by OTP; help wanted from the community label May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug help wanted Issue not worked on by OTP; help wanted from the community priority:low team:PS Assigned to OTP team PS
Projects
None yet
Development

No branches or pull requests

4 participants