SUMMARY If I enable Kate's LSP plugin, then closing the window takes 0.7 seconds for the Kate process to terminate, and Ctrl+Q or deactivating the plugin causes the app to hang for 0.7 seconds. STEPS TO REPRODUCE 1. Open Konsole, type kate, press Enter. There should only be one window with an empty document. 2. In Configure Kate -> Plugins, check "LSP Client" and press OK. 3. Close Kate using the title bar button, or using Ctrl+Q. (This hang is reproducible every time you restart Kate and close it.) Alternatively: 1. Open Kate. 2. In Configure Kate -> Plugins, check "LSP Client". - Another bug: checking/unchecking plugins takes effect immediately. Cancel does *not* revert changes to the set of active plugins, and neither does Cancel then closing+reopening Kate. 3. Uncheck "LSP Client". (This hang is reproducible every time you uncheck the checkbox.) OBSERVED RESULT Whenever the LSP client is being unloaded, Kate hangs for 700ms. - When closing with the title bar button, the window closes first, but the process lingers around for 700ms before returning control to the terminal. - When quitting with Ctrl+Q, the Kate windows hang for 700ms, unresponsive, before disappearing (at which point the process terminates quickly). - When deactivating the LSP Client plugin, the Configure Kate modal dialog hangs for 700ms before unchecking. (The main Kate window is already unresponsive because the dialog is modal.) EXPECTED RESULT Kate does not hang when unloading the LSP client. SOFTWARE/OS VERSIONS Operating System: Arch Linux KDE Plasma Version: 5.22.1 KDE Frameworks Version: 5.83.0 Qt Version: 5.15.2 Kernel Version: 5.12.10-zen1-1-zen (64-bit) Graphics Platform: X11 Processors: 12 × AMD Ryzen 5 5600X 6-Core Processor Memory: 15.6 GiB of RAM Graphics Processor: NVIDIA GeForce GT 730/PCIe/SSE2 ADDITIONAL INFORMATION
I investigated the LSP client. Pressing Ctrl+C in gdb during the hang pointed within LSPClientServerManagerImpl::~LSPClientServerManagerImpl(). The corresponding function as of today's master is https://github.com/KDE/kate/blob/45362eaa64ede7b0dbb172a5c154859c76fd6231/addons/lspclient/lspclientservermanager.cpp#L213, which matches my local 21.04.2 function. According to the source, Kate's LSP client is designed to sleep the UI thread for 500ms (using QThread::msleep(500)) after sending a termination request to LSPs, before sending SIGTERM and SIGKILL (each with a 100ms delay afterwards). I'm not sure if waiting for language servers to terminate is a bad design, or if blocking the UI thread is a bad design, or what would be better. However, the current design is implemented poorly, since it sleeps for 700ms even though `m_servers` is empty (for example, in an empty Kate window without open files)! Also there's some oddities going on, with `connect(s.data(), &LSPClientServer::stateChanged, this, handler)`, a `QTimer t` that never fires, and a `QEventLoop q` that seems to go unused aside from calling quit() on it. I don't understand what `count` is doing either. Maybe it can be removed entirely (but I don't know what to do with `handler`). I think it's dead code that used to be active: https://github.com/KDE/kate/commit/724a9d0af0e9b4ec04d91f74ca9bbc9dbeaa2ee2 Additionally, the current code checks for `auto &s = si.server; if (!s)` to determine whether to request shutdown, SIGTERM, then SIGKILL. I don't think `ServerInfo { QSharedPointer<LSPClientServer> server` will become a null pointer when the LSPClientServer shuts down (but I didn't verify it doesn't get overwritten). Assuming it doesn't become a null pointer, that means Kate calls LSPClientServer::stop() (which calls QProcess::terminate()/kill()) even if the process is already dead! ---- An easy partial fix is to skip the m_servers/msleep() song and dance entirely if m_servers is empty. This will fix the shutdown lag if no language servers are running. If maintainers want to keep the current overall behavior (blocking the UI thread while sending terminate+SIGTERM+SIGKILL), I think a better fix is: - If any servers are active, send a termination request, "join" on them with a 500ms timeout, then remove the servers which have shutdown. - If any servers are left, send SIGTERM, "join" on them with a 100ms timeout, then remove the servers which have shutdown. - If any servers are left, send SIGKILL and don't bother joining anymore. If they don't shutdown from SIGKILL, there's nothing we can do (I think). If you feel like it, you can send the user a popup if LSPs don't respond to termination, SIGTERM, or SIGKILL. Issue is, I don't know how to properly implement "joining", and haven't researched how to do it yet. I can look into it if the maintainers like my proposed control flow. ---- While researching this problem, I was exploring the various Kate plugin APIs, and have some comments about the design. `LSPClientServer::stop(int to_term_ms, int to_kill_ms)` is unintuitive to me, but I don't think it can be significantly improved (aside from using a constant like DONT_TERMINATE instead of -1), since all combinations of {-1, >=0} for the two parameters are used (in ~LSPClientServerPrivate() and ~LSPClientServerManagerImpl(). I also found `LSPClientServerManagerImpl::restart(const ServerList &servers)` (https://github.com/KDE/kate/blob/45362eaa64ede7b0dbb172a5c154859c76fd6231/addons/lspclient/lspclientservermanager.cpp#L392). Instead of waiting for the old process to terminate, it sends a termination request immediately, SIGTERM in 0.4 seconds, SIGKILL in 0.8 seconds, and emits serverChanged() (which somehow respawns the server) in 1.2 seconds. This design is slower and arguably less elegant than waiting for the processes to terminate, but simpler and has more predictable behavior. You can watch the LSP servers stop and start by running `watch -n0.1 'pstree -a $(pgrep kate)'` and triggering LSP restarts in Kate. - Interestingly, if you restart the LSP, then press "jump to definition" before clangd starts up, it fails, but instantly spawns a clangd so pressing "jump to definition" a second time works. - By mapping a shortcut to "Restart All LSP Servers" and holding the shortcut down, you can get Kate to start close to 10 clangd processes at once. I don't know if this is a problem, or how to fix it.
A possibly relevant merge request was started @ https://invent.kde.org/utilities/kate/-/merge_requests/440
Git commit e2fb2fb47288ff6467ebb73e6a33caf782f68f3a by Christoph Cullmann, on behalf of Mark Nauwelaerts. Committed on 22/06/2021 at 17:06. Pushed by cullmann into branch 'master'. lspclient: bypass shutdown delay if not needed and cleanup defunct code M +10 -13 addons/lspclient/lspclientservermanager.cpp https://invent.kde.org/utilities/kate/commit/e2fb2fb47288ff6467ebb73e6a33caf782f68f3a