Avoid creating documents immediately because there could be a modal dialog open. Modal code often assumes the current doc hasn't changed and calls `document_get_current` at different times.
Fixes #2599. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3395
-- Commit Summary --
* Open CLI docs only once main window is active
-- File Changes --
M src/socket.c (21)
-- Patch Links --
https://github.com/geany/geany/pull/3395.patch https://github.com/geany/geany/pull/3395.diff
When a file is opened from a remote instance while a modal is open, the `handle_input_filename()` is called as fast as the main loop allows. This causes high CPU usage until the main window gets the focus back and the idle function is removed.
I'd say this doesn't make it better than before especially as it is most probably not clear to the user why Geany suddently eats the CPU.
@ntrel pushed 1 commit.
e3de0579b9878f503b952a77cc2b067eca60e74b Use 100ms timeout to prevent idle callback spinning
Certainly better than allowing it, but doesn't this break the use case of #2599?
1. Open a document 2. File->Save As 3. Notice an interesting file in the list of files in the dialog 4. Open interesting file from the command-line (in another program) with geany interesting-file. The file does not open (because the dialog is still open). 5. User says WHAT THE!!!! 6. complete save as 7. file opens 8. User says WHAT THE!!!!
doesn't this break the use case of https://github.com/geany/geany/issues/2599?
No:
The file does not open (because the dialog is still open).
User says WHAT THE!!!! complete save as file opens User says WHAT THE!!!!
Which is exactly what happens now visually (but not behind the scenes), because the dialog is modal.
@kugel- commented on this pull request.
@@ -688,7 +698,10 @@ gboolean socket_lock_input_cb(GIOChannel *source, GIOCondition condition, gpoint
if (buf_len > 0 && buf[buf_len - 1] == '\n') buf[buf_len - 1] = '\0';
- handle_input_filename(buf); + // Important: + // avoid creating documents now because there could be a modal dialog open. + // modal code may call document_get_current and assume it hasn't changed + g_timeout_add(100, handle_input_filename, g_strdup(buf));
Interesting that `g_idle_add()` is problematic. I thought idle callbacks aren't run when a model dialog is open (because the API is synchornous) but I guess the API runs a separate mainloop internally which runs idle callbacks (and the GIO input callback here...).
100ms is a bit look for my taste. These could be user visible. I think 30ms could do the job as well without causing CPU problems. But I guess not a big deal for this niche problem.
@kugel- approved this pull request.
@elextr commented on this pull request.
@@ -688,7 +698,10 @@ gboolean socket_lock_input_cb(GIOChannel *source, GIOCondition condition, gpoint
if (buf_len > 0 && buf[buf_len - 1] == '\n') buf[buf_len - 1] = '\0';
- handle_input_filename(buf); + // Important: + // avoid creating documents now because there could be a modal dialog open. + // modal code may call document_get_current and assume it hasn't changed + g_timeout_add(100, handle_input_filename, g_strdup(buf));
but I guess the API runs a separate mainloop internally which runs idle callbacks (and the GIO input callback here...).
Modal dialogs are only specified to freeze the user input to the UI, so any other event the mainloop sees should "just work".
The file does __not__ open (because the dialog is still open). ... emphasis added
@ntrel I thought from your description without this PR the file __does__ open, behind the dialog.
Anyway, as I said this is better than having ill defined effects from Geany's non-reentrancy, its so edge case nobody else has complained AFAIK, so that if you are happy with the result then ok by me.
I don't really have a set opinion on this, but one downside I see is that thus loading a large file could potentially feel slower, because the main window would pop up *before* it gets loaded. Also, note that this affects virtually *all* CLI openings, as the main window is unlikely to be focused when that happens.
I get that this tries to "fix the problem everywhere", but is it the only possible reason for this to happen? And if it's not bulletproof, can't we "just" fix the offending case(s) not to be buggy in that regard? (using the document ID or document pointer, etc.)
this affects virtually all CLI openings, as the main window is unlikely to be focused when that happens.
Maybe that could be worked around by checking if the main window has focus, not sure.
can't we "just" fix the offending case(s) not to be buggy in that regard? (using the document ID or document pointer, etc.)
Well the problem is finding where the bugs are ;-)
This fixes the only one I'm aware of: https://github.com/geany/geany/pull/3415
github-comments@lists.geany.org