I realized I will need this signal for the LSP plugin as I ran into issues when Geany is shutting down. I'll try to explain what is going on here:
1. The jsonrpc-glib plugin uses glib asynchronous operations which require the main loop to run to send/receive messages. 2. When Geany is quitting, the main loop has no chance to run while https://github.com/geany/geany/blob/4c1191ac442e1bcceb60d7c2497e0baf5d5f4fa9... is being executed. 3. The LSP plugin needs to terminate the LSP server processes gracefully when Geany is quitting by sending "shutdown" requests (and waiting for responses from the servers) followed by "exit" notifications. This currently happens inside plugin_cleanup() but because of (1) and (2) it has to run the main loop by itself (https://github.com/techee/geany-lsp/blob/3a31ec9be8323c668299d1c292b7c401f2e...) 4. However, the side effect of running the main loop is that other events originating from Geany or plugins (such as various idle functions or timers) are processed by the main loop. These would normally never get executed and lead to unexpected things during shutdown (I did run into strange problems with #3911 because of this).
The bad thing here is that the main loop is run inside plugin_cleanup() and lots of things happen before it - projects and documents are closed (which emits lots of signals to which Geany or plugins react when the main loop is running), plugins are unloaded, etc.
To fix this, I'd like to move LSP server termination (and manual main loop running) to the point which is closer to the point where normal main loop runs and which is "calmer" and not affected by all the stuff happening during the shutdown process. This is the point where the "geany-before-quit" was added and the main loop would be run from its handler by the plugin. Even if some Geany events are still executed from it, it should be "normal" non-shutdown events that would get executed by the normal main loop too.
(Note that the fact that the main loop is running means that until all LSP servers are terminated, it would be possible to interact with the GUI of Geany. In practice though the termination is very fast so this isn't a real-world problem.) You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/4069
-- Commit Summary --
* Add the geany-before-quit signal emitted before Geany is quitting * Remove misleading main_status.quitting assignment
-- File Changes --
M doc/pluginsignals.c (10) M src/geanyobject.c (6) M src/geanyobject.h (1) M src/libmain.c (4) M src/plugindata.h (2)
-- Patch Links --
https://github.com/geany/geany/pull/4069.patch https://github.com/geany/geany/pull/4069.diff
@b4n commented on this pull request.
@@ -1287,8 +1289,6 @@ static gboolean do_main_quit(void)
geany_debug("Quitting...");
- main_status.quitting = TRUE;
Why?
This sounds like a useful signal, but in your specific case wouldn't a separate communication thread be better?
@techee commented on this pull request.
@@ -1287,8 +1289,6 @@ static gboolean do_main_quit(void)
geany_debug("Quitting...");
- main_status.quitting = TRUE;
Because it's already set in https://github.com/geany/geany/blob/4264d0c507abc32c4e9f13aebaaec53d0c9e376b...
and the extra assignment here is confusing and makes the impression that
``` configuration_save(); project_close(); document_close_all(); ```
above it are made without this flag set.
This sounds like a useful signal, but in your specific case wouldn't a separate communication thread be better?
I could probably run a separate main loopon a different thread (like described here https://developer.gnome.org/documentation/tutorials/main-contexts.html) but this would really complicate all the communication and add a lot of code as I'd have to pass all the communication data to and from this thread.
gnome-builder which uses jsonrpc-glib for LSP does this using the default main queue on the main thread as well (but at least from what I have seen, they don't properly terminate the LSP servers and don't send the "exit" notifications)
So yeah, I wanted to add something to Geany that might make sense for other plugins too but what I could use for this specific use case as well.
@b4n OK to merge? Even if I decided to use some extra threads (which I doubt, best to avoid those), I'd still need to know that Geany is terminating to perform the cleanup. So I'd need the signal anyway.
LGTM
@techee pushed 2 commits.
f4e1d54e002504a685976e0162fd627d76d1dc48 Add the geany-before-quit signal emitted before Geany is quitting 519c7038862ec39027c6b61061043f335b1b5b47 Remove misleading main_status.quitting assignment
I just re-pushed with added `@since` in documentation and incremented API version.
Merging now.
Merged #4069 into master.
github-comments@lists.geany.org