Bug 492998 - Jobs are not canceled when KNS dialog is closed, which can cause initiating apps to not exit
Summary: Jobs are not canceled when KNS dialog is closed, which can cause initiating a...
Status: RESOLVED FIXED
Alias: None
Product: frameworks-knewstuff
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: HI major
Target Milestone: ---
Assignee: Dan Leinir Turthra Jensen
URL:
Keywords:
: 491058 495298 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-09-12 04:44 UTC by Nate Graham
Modified: 2024-11-12 15:34 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In: 6.8
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Graham 2024-09-12 04:44:22 UTC
Everything KDE built from today's git master.


STEPS TO REPRODUCE
1. In a terminal window, launch `systemsettings`
2. Navigate to the Colors KCM
3. Click "Get New"
4. Don't do anything, just close the KNS window
5. Close the System Settings window


OBSERVED RESULT
The executable continues running, presumably because there's a KNS job that's still running or stuck. This prevents re-opening System Settings since it's a single-instance app.


EXPECTED RESULT
System Settings fully quits.


ADDITIONAL INFORMATION
Potentially relevant log warning:

kf.newstuff.quick.private: You have not set a transientParent on KNewStuff.Dialog or .Action. This may cause severe problems with window and lifetime management. We'll try to fix the situation automatically but you should really provide an explicit transientParent
Comment 1 Harald Sitter 2024-09-20 11:46:07 UTC
Curiously I cannot reproduce this with my hacked up knewstuff so I think I fixed this by accident somewhere already, just need to find the relevant diff.

Definitely broken with stock master build though.
Comment 2 Harald Sitter 2024-09-20 11:46:44 UTC
Actually. Can you give this a go https://invent.kde.org/frameworks/knewstuff/-/merge_requests/333
Comment 3 Nate Graham 2024-09-20 14:43:37 UTC
Unfortunately that MR does not fix the issue for me. A thread is still open and blocking systemsettings from exiting.
Comment 4 Harald Sitter 2024-09-23 10:29:40 UTC
Ah! I have more diff on my system. Will test some more.

I have a conceptual question meanwhile: what if installation takes more than a second or two?
Comment 5 Nate Graham 2024-09-23 13:47:02 UTC
I see two UX-feasible options for how to handle if the user tries to close the window while a job is pending:
- Show a warning dialog with the option to kill the job to close the window, or else keep it open until it finished
- Allow closing the window and show a system notification with the progress of the job (Discover does this)
Comment 6 Harald Sitter 2024-10-14 11:42:34 UTC
Git commit 271c92e978b1a9637d043faff2fd3b15121864b1 by Harald Sitter.
Committed on 14/10/2024 at 11:39.
Pushed by sitter into branch 'master'.

filecopyworker: try to gracefully quit the thread. then terminate it

this should prevent scenarios where the thread gets destroyed on
application exit and bringing down the application with it due to
destruction of a running thread being qFatal.

M  +12   -1    src/core/jobs/filecopyworker.cpp

https://invent.kde.org/frameworks/knewstuff/-/commit/271c92e978b1a9637d043faff2fd3b15121864b1
Comment 7 Harald Sitter 2024-10-14 11:42:36 UTC
Git commit c7a457ea5ffe2c13b853a0230295ab36cefa609a by Harald Sitter.
Committed on 14/10/2024 at 11:39.
Pushed by sitter into branch 'master'.

put qnetworkreplys in a self-aborting unique_ptr

this should prevent issues where the reply keeps running for a
potentially longer time than desired. when the worker is gone there is
no need to run the reply any longer.

M  +19   -8    src/core/jobs/httpworker.cpp

https://invent.kde.org/frameworks/knewstuff/-/commit/c7a457ea5ffe2c13b853a0230295ab36cefa609a
Comment 8 Harald Sitter 2024-10-14 11:42:42 UTC
Git commit 93667cb0fe8cfca5b1b85e957d3cd5324ca48fc0 by Harald Sitter.
Committed on 14/10/2024 at 11:39.
Pushed by sitter into branch 'master'.

parent the xml loader's httpjob

so it may be destroyed when the loader gets destroyed and not keep
running for potentially more than a second (holding up app shutdown)

M  +1    -1    src/core/xmlloader.cpp

https://invent.kde.org/frameworks/knewstuff/-/commit/93667cb0fe8cfca5b1b85e957d3cd5324ca48fc0
Comment 9 Harald Sitter 2024-10-14 11:42:44 UTC
Git commit d31ed6cccc30091754501476e18fa3b2efe62903 by Harald Sitter.
Committed on 14/10/2024 at 11:39.
Pushed by sitter into branch 'master'.

make sure the action's dialog closes

because it is held by a loader it would ordinarily stick around which is
problematic because it is a top level window and can hold up automatic
application quitting on last-window-closed

M  +8    -0    src/qtquick/qml/Action.qml

https://invent.kde.org/frameworks/knewstuff/-/commit/d31ed6cccc30091754501476e18fa3b2efe62903
Comment 10 Nate Graham 2024-10-28 03:23:31 UTC
*** Bug 491058 has been marked as a duplicate of this bug. ***
Comment 11 Nate Graham 2024-10-28 03:23:43 UTC
*** Bug 495327 has been marked as a duplicate of this bug. ***
Comment 12 Nate Graham 2024-10-28 04:08:24 UTC
*** Bug 495298 has been marked as a duplicate of this bug. ***
Comment 13 ttthwvezrjdmeldtud 2024-11-12 03:10:34 UTC
(In reply to Nate Graham from comment #11)
> *** Bug 495327 has been marked as a duplicate of this bug. ***

I only have this issue when I actually USE the theme, not just selecting "get new"
Comment 14 Nate Graham 2024-11-12 03:18:39 UTC
Your issue sounds different; please open a new bug report.
Comment 15 Nate Graham 2024-11-12 14:44:56 UTC
*** Bug 495327 has been marked as a duplicate of this bug. ***