WIP: Fix failing 'Close GUI' and 'Quit GRASS' on macOS (trac #3009) #408

nilason · 2020-03-10T12:35:36Z

This addresses the issue trac#3009.

Consists of two parts:

fix for "Quit GRASS"
quitting will not quit the shell on mac, only the grass process
fix for "Close GUI"
for some reason atexit for gui/wxpython/wxgui.py throws error on mac, implementation of cleanup has been altered

Has been tested on mac and win.

addresses https://trac.osgeo.org/grass/ticket/3009

petrasovaa

I will comment only on wxgui.py, it looks fine, although it would be good to know what error it gives you.

Please remove unused import atexit.

wenzeslaus · 2020-03-26T01:34:35Z

lib/init/grass.py

+        # Make PID for this process on macOS, needed for enable quitting
+        # GRASS, trac #3009.
+        if MACOSX:
+            kv['PID'] = str(os.getpid())
        write_gisrc(kv, gisrc)
        exit_val = shell_process.wait()
        if exit_val != 0:


The line close_gui() and onward are not reached on macOS when exiting from GUI (no problem for close_gui(), but problem especially for clean_all()).

close_gui() is actually important too if there is more than one GUI running.

wenzeslaus · 2020-03-26T02:26:07Z

Hi @nilason, I can't comment on your original self._quitGRASS() comment, but based on your changes, I would say that it is the shell process which is not accepting the signal. The grass.py starts a shell itself and that's the shell which is then propagated through PID "gis var" to GUI(s). This is the shell which is not accepting the signal.

Perhaps you can test if this is the case with kill program in command line and whatever shell GRASS is using on your machine (I mean the one you see in the welcome message in command line under the large GRASS GIS text).

As for changes in grass.py, the standard behavior is terminating shell (with typing exit (or whatever is appropriate) by user or by sending SIGTERM signal to the shell process). grass.py then continues (after shell_process.wait()) and does some clean up procedures. These are there explicitly (and not with something like atexit.register()) partially for historical reasons and partially perhaps to be simply explicit about what happens and in what order, but perhaps something to be revised (atexit.register() is used at other places in grass.py - definitively in some parts I touched).

In other words, with the current grass.py and your PR on macOS, when the GUI exists, instead of asking the terminal to quit and continuing in grass.py, the grass.py is terminated right away.

fix for "Quit GRASS"
quitting will not quit the shell on mac, only the grass process

If I understand all this correctly, if you start GRASS GIS from existing shell, quitting GRASS, should not exit the parent shell. The - very common - exception is when the system is starting a terminal window just for GRASS GIS (and I don't know about it enough). However, the more transparent case is with an existing terminal and there typing exit in the shell translates to exiting GRASS GIS from user point of view, but nothing beyond that. Here is an example session:

$ # in my shell now
$ grass --text  # starting GRASS GIS
Cleaning up temporary files...
Starting GRASS GIS...  [these come from grass.py]

          __________  ___   __________    _______________
         / ____/ __ \/   | / ___/ ___/   / ____/  _/ ___/
        / / __/ /_/ / /| | \__ \\_  \   / / __ / / \__ \
       / /_/ / _, _/ ___ |___/ /__/ /  / /_/ // / ___/ /
       \____/_/ |_/_/  |_/____/____/   \____/___//____/
...
This version running through:            Bash Shell (/bin/bash)
...
GRASS 7.6.1 (nc_spm):~ > # now in the shell started by grass.py
GRASS 7.6.1 (nc_spm):~ > exit  # exit the shell
exit



Cleaning up temporary files...
Done.

Goodbye from GRASS GIS  [these come from grass.py again]

$ # back in my original shell again

Well, needless to say, let me know your thoughts on this and whether you observe this behavior.

nilason · 2020-03-26T11:17:58Z

Thank you both very much for looking into this.

You gave me quite a few things to think on. I need to reconsider the implementation of this PR, so please hold on.

For a start I couldn't reproduce the atexit problem, now using standard Python instead of a debug version. My proposed changes to wxgui.py are therefore not necessary.
(Note to self: always double check with standard Python!!).

nilason · 2020-03-27T21:36:48Z

Now I know how to reproduce the atexit issue. It wasn't the python debug build causing this, but setting grass debug variable to 1+.
The gui quits, but without cleanup. With my fix to wxgui.py this is no longer a problem.

$ grass79 --text
$ g.gisenv set="DEBUG=1"
$ g.gui wxpython

# QUIT GUI (through menu or shortcut)
Error in atexit._run_exitfuncs:
$ exit
Unable to close GUI. [Errno 3] No such process

Is this a mac specific behaviour?

wenzeslaus · 2020-03-31T01:36:36Z

I can reproduce this (Error in atexit._run_exitfuncs: [no further message] and Unable to close GUI. [Errno 3] No such process) on Linux in recent 7.9. I can't reproduce this with 7.6.

nilason · 2020-03-31T18:41:19Z

Yes @wenzelaus, the shell isn't responding to the signal (os.kill(PID, SIGTERM)). That's why I made the modifications the way I did and you are also right that it doesn't do any clean up. So I'm (still) looking for acceptable solution for this. Meanwhile let me present some of the findings of how the processes respond to signals (from shell):

$ grass79
Starting GRASS GIS...
[...]
shell_process: 4963
os.getpid(): 4959 # this is the grass process
grass > kill -s TERM 4963
grass > echo $?
0
grass > ps aux | grep -i 4963
[...]
nila           4963   0.0  0.0  4288476   1820 s002  S     2:31PM   0:00.02 /bin/bash
grass > kill -s TERM 4959
Terminated: 15
$ logout
Saving session...GRASS 7.9.dev (newLocation):/ > exit

...copying shared history...
...saving history...truncating history files...
...completed.

[Process completed]

This is a normal exit session:

$ grass79
Starting GRASS GIS...
[...]
shell_process: 41365
os.getpid(): 41361 # this is the grass process
grass > exit
exit
Cleaning up default sqlite database ...
Cleaning up temporary files...
Done.

Goodbye from GRASS GIS

$

The only signal the shell_process responds to is SIGKILL, which is not acceptable.

nilason · 2020-04-07T14:29:52Z

It turns out that manually adding a trap in bash, makes "GRASS quit" work on mac without any alterations to lib/init/grass.py. But I don't know how to make use of this info programmatically, neither in python nor perhaps in https://github.com/OSGeo/grass/blob/master/lib/init/run.c. I'd really appreciate some help.

$ grass79
Starting GRASS GIS...
[...]

Launching <wxpython> GUI in the background, please wait...
grass > trap "exit" TERM

# "Quit GRASS GIS" in GUI

Cleaning up default sqlite database ...
Cleaning up temporary files...
Done.

Goodbye from GRASS GIS

wenzeslaus · 2020-04-07T17:35:22Z

From my search, esp. this Unix SE, it seems that shells often ignore SIGTERM when interactive. Do you understand this the same way?

As for run.c, I'm not familiar with it and it seems to go in a different direction than what I say above. Or perhaps it is trying to behave as an interactive shell. The documentation is not clear to me. I don't understand the last part of the documentation (There is no way to tell the user's shell to re-activate interrupts in shell-ese.). run.c is so old (and stable) that it does not have any major modifications except for one for MINGW 14 years ago. Did you try removing run.c to see if it has any influence, good or bad?

nilason · 2020-04-07T18:55:46Z

From my search, esp. this Unix SE, it seems that shells often ignore SIGTERM when interactive. Do you understand this the same way?

Thanks! Yes, that should be the cause of the problem. And makes the run.c aspect unrelated (I didn't quite understand what it was doing, but I could see it was poking with signals...).

Now, there only remains to find a solution to this. I did come up with a solution with SIGHUP, which the shell process responds to. It did all cleanup, but left the original shell in a non-acceptable state.

wenzeslaus · 2020-04-17T14:58:57Z

Which shell are you using? I get the same behavior with tcsh on Linux.

tcsh ignores SIGTERM (silently) and also Ctrl+D (with a message) :

$ tcsh
$ ps
  PID TTY          TIME CMD
...
32525 pts/27   00:00:00 tcsh
$ # in another shell
$ kill -SIGTERM 32525

$ # in tcsh already (but that's not needed)
$ setenv SHELL /usr/bin/tcsh
$ grass79
GRASS GIS 7.9.dev > ^D
Use "exit" to leave tcsh.

nilason · 2020-04-17T16:02:35Z

It is the default shell with macOS 14: bash.
From macOS 15 it changed to zsh, but while I have no such system at hand to test on, a similar test of yours with zsh was also silently ignored.

CTRL+d seems to work though.

cmbarton · 2020-04-17T18:07:07Z

It is the default shell with macOS 14: bash.
From macOS 15 it changed to zsh, but while I have no such system at hand to test on, a similar test of yours with zsh was also silently ignored.

CTRL+d seems to work though.

I've updated by MacBook to OSX 15 and can test this once it is merged in and I can recompile. This is master?

nilason · 2020-04-17T18:30:47Z

I've updated by MacBook to OSX 15 and can test this once it is merged in and I can recompile. This is master?

There is no acceptable fix yet, what is in the PR now is not a good fix. What you can test on macOS 15, is whether quitting (from GUI) fails on master still. I assume it will.

cmbarton · 2020-04-17T18:41:20Z

I've updated by MacBook to OSX 15 and can test this once it is merged in and I can recompile. This is master?

There is no acceptable fix yet, what is in the PR now is not a good fix. What you can test on macOS 15, is whether quitting (from GUI) fails on master still. I assume it will.

OK. Master as of what date? Do I need to recompile after 4 April?

nilason · 2020-04-17T19:27:26Z

No, any recent build would do, master, 7.8. Now, what is interesting is if macOS 15 makes any difference.

cmbarton · 2020-04-17T19:29:25Z

No, any recent build would do, master, 7.8. Now, what is interesting is if macOS 15 makes any difference.

I've already run my 4 April build on the MacBook. I had to quit the same way as before, from the terminal. I can double check though.

nilason · 2020-04-18T08:08:35Z

@lbartoletti Is this an issue (see trac#3009) on FreeBSD as well? Or does quitting from GUI work as expected?

cmbarton · 2020-04-20T18:37:58Z

No, any recent build would do, master, 7.8. Now, what is interesting is if macOS 15 makes any difference.

I've already run my 4 April build on the MacBook. I had to quit the same way as before, from the terminal. I can double check though.

Just an update. I did launch GRASS 7.9 on my MacBook with OSX 10.15. The file menu only says "Quit GUI". When I select it, it does quit the GUI and leaves you in the terminal. There is no option to quit GRASS, which did not work before. So nothing really to test.

nilason · 2020-04-20T19:10:23Z

Just an update. I did launch GRASS 7.9 on my MacBook with OSX 10.15. The file menu only says "Quit GUI". When I select it, it does quit the GUI and leaves you in the terminal. There is no option to quit GRASS, which did not work before. So nothing really to test.

It's under Python menu > Quit Wxgui, alternatively Cmd+Q.

cmbarton · 2020-04-20T19:22:09Z

Just an update. I did launch GRASS 7.9 on my MacBook with OSX 10.15. The file menu only says "Quit GUI". When I select it, it does quit the GUI and leaves you in the terminal. There is no option to quit GRASS, which did not work before. So nothing really to test.

It's under Python menu > Quit Wxgui, alternatively Cmd+Q.

I did that one too. It quit the GUI. No option to quit GRASS though. I just landed back in the terminal.

nilason · 2020-04-20T19:39:52Z

There should be tree choices: Cancel, Close GUI and Quit GRASS GIS. The last one should close GUI and exit GRASS – that's the one in question which doesn't work (only closes GUI).

cmbarton · 2020-04-20T19:59:56Z

There should be tree choices: Cancel, Close GUI and Quit GRASS GIS. The last one should close GUI and exit GRASS – that's the one in question which doesn't work (only closes GUI).

I did not see the tree choices. I'll look again tonight.

nilason · 2020-04-20T20:14:05Z

I did not see the tree choices.

that should have been three choices or alternatives (no apple trees there:)

cmbarton · 2020-04-20T20:57:20Z

I did not see the tree choices.

that should have been three choices or alternatives (no apple trees there:)

;-) I thought you mean a choice tree. But if you mean a dialog should pop up with the 3 alternative buttons, it doesn't. If I click that menu choice, it just closes the GUI.

This is a version I compiled on 1 April.

nilason · 2020-04-20T21:57:29Z

It's under Python menu > Quit Wxgui, alternatively Cmd+Q.

I'm sure the three-button dialog is there, not under File-menu, but under Python-menu.

cmbarton · 2020-04-26T19:23:53Z

It's under Python menu > Quit Wxgui, alternatively Cmd+Q.

I'm sure the three-button dialog is there, not under File-menu, but under Python-menu.

I rechecked this weekend. You are right. And selecting Quite GRASS just dumps you into the terminal, but does not quit GRASS.

lbartoletti · 2020-04-27T13:23:29Z

It's under Python menu > Quit Wxgui, alternatively Cmd+Q.

I'm sure the three-button dialog is there, not under File-menu, but under Python-menu.

I rechecked this weekend. You are right. And selecting Quite GRASS just dumps you into the terminal, but does not quit GRASS.

@lbartoletti Is this an issue (see trac#3009) on FreeBSD as well? Or does quitting from GUI work as expected?

@cmbarton @nilason Yes, it returns on the terminal also on FreeBSD

nilason · 2020-04-27T13:43:39Z

Yes, it returns on the terminal also on FreeBSD

@lbartoletti Thanks for the info!

nilason · 2020-07-16T06:51:49Z

This PR is overridden by #722 and #772 and the issues have been resolved. Quitting GRASS from GUI should work with bash or zsh terminal shell.

fix failing 'Close GUI' and 'Quit GRASS' on macOS

0c9a425

addresses https://trac.osgeo.org/grass/ticket/3009

petrasovaa requested review from petrasovaa and wenzeslaus March 10, 2020 19:48

neteler added the macOS macOS specific label Mar 25, 2020

nilason mentioned this pull request Mar 25, 2020

Fix failing vector digitizer on mac, trac#3487 #456

Merged

petrasovaa requested changes Mar 26, 2020

View reviewed changes

wenzeslaus reviewed Mar 26, 2020

View reviewed changes

nilason changed the title ~~Fix failing 'Close GUI' and 'Quit GRASS' on macOS (trac #3009)~~ WIP: Fix failing 'Close GUI' and 'Quit GRASS' on macOS (trac #3009) Apr 5, 2020

This was referenced Jul 7, 2020

Add zsh terminal prompt support #722

Merged

wxGUI: fix atexit error in debug mode #772

Merged

nilason closed this Jul 16, 2020

nilason deleted the fix-trac-3009 branch July 19, 2020 11:59

neteler added this to the 8.0.0 milestone Dec 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Fix failing 'Close GUI' and 'Quit GRASS' on macOS (trac #3009) #408

WIP: Fix failing 'Close GUI' and 'Quit GRASS' on macOS (trac #3009) #408

nilason commented Mar 10, 2020

petrasovaa left a comment

wenzeslaus Mar 26, 2020

wenzeslaus Mar 26, 2020

wenzeslaus commented Mar 26, 2020

nilason commented Mar 26, 2020

nilason commented Mar 27, 2020

wenzeslaus commented Mar 31, 2020

nilason commented Mar 31, 2020

nilason commented Apr 7, 2020

wenzeslaus commented Apr 7, 2020

nilason commented Apr 7, 2020

wenzeslaus commented Apr 17, 2020

nilason commented Apr 17, 2020 •

edited

Loading

cmbarton commented Apr 17, 2020

nilason commented Apr 17, 2020

cmbarton commented Apr 17, 2020

nilason commented Apr 17, 2020

cmbarton commented Apr 17, 2020

nilason commented Apr 18, 2020

cmbarton commented Apr 20, 2020

nilason commented Apr 20, 2020

cmbarton commented Apr 20, 2020

nilason commented Apr 20, 2020

cmbarton commented Apr 20, 2020

nilason commented Apr 20, 2020

cmbarton commented Apr 20, 2020

nilason commented Apr 20, 2020

cmbarton commented Apr 26, 2020

lbartoletti commented Apr 27, 2020

nilason commented Apr 27, 2020

nilason commented Jul 16, 2020

WIP: Fix failing 'Close GUI' and 'Quit GRASS' on macOS (trac #3009) #408

WIP: Fix failing 'Close GUI' and 'Quit GRASS' on macOS (trac #3009) #408

Conversation

nilason commented Mar 10, 2020

petrasovaa left a comment

Choose a reason for hiding this comment

wenzeslaus Mar 26, 2020

Choose a reason for hiding this comment

wenzeslaus Mar 26, 2020

Choose a reason for hiding this comment

wenzeslaus commented Mar 26, 2020

nilason commented Mar 26, 2020

nilason commented Mar 27, 2020

wenzeslaus commented Mar 31, 2020

nilason commented Mar 31, 2020

nilason commented Apr 7, 2020

wenzeslaus commented Apr 7, 2020

nilason commented Apr 7, 2020

wenzeslaus commented Apr 17, 2020

nilason commented Apr 17, 2020 • edited Loading

cmbarton commented Apr 17, 2020

nilason commented Apr 17, 2020

cmbarton commented Apr 17, 2020

nilason commented Apr 17, 2020

cmbarton commented Apr 17, 2020

nilason commented Apr 18, 2020

cmbarton commented Apr 20, 2020

nilason commented Apr 20, 2020

cmbarton commented Apr 20, 2020

nilason commented Apr 20, 2020

cmbarton commented Apr 20, 2020

nilason commented Apr 20, 2020

cmbarton commented Apr 20, 2020

nilason commented Apr 20, 2020

cmbarton commented Apr 26, 2020

lbartoletti commented Apr 27, 2020

nilason commented Apr 27, 2020

nilason commented Jul 16, 2020

nilason commented Apr 17, 2020 •

edited

Loading