[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 Anthony Fieroni changed: What|Removed |Added CC||[email protected] --- Comment #26 from Anthony Fieroni --- (In reply to Harald Sitter from comment #14) > Multiple "fronting" kdirwatch objects may point to the same internal private > instance that actually encapsulates the watching logic. This is so we can > install 300 kdirwatches on plasmarc but in the end only a single inotify > watch is used to back all those fronting objects. What's the reason behind that? -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 --- Comment #25 from Harald Sitter --- Git commit e40eac96d2258b8b8dd8589aa3c0bbe4e8c64d05 by Harald Sitter. Committed on 13/11/2023 at 14:02. Pushed by sitter into branch 'kf5'. kdirwatch: don't crash after moving threads if a user moves a public watch instance across threads we used to have a d but now local data (thereby not running the dtor) and so eventually the private watch in the old thread would try to emit data into a since deleted public watch in the new thread to prevent this problem we'll "brick" the watch upon moving threads. specifically we'll fully unregister it with the old private and create/apply a new private in the new thread. since we currently have no way to reliably copy Entry as part of this we cannot really replicate the correct watches on the new thread. definitely something that could be made to work in the future though. in the meantime critically log that moving threads does not do what you expect it to do when used with kdirwatch. (cherry picked from commit 2fc44a3c300324c614f2d3b86fb465f2c413c70c) M +31 -0autotests/kdirwatch_unittest.cpp M +30 -0src/lib/io/kdirwatch.cpp M +5-0src/lib/io/kdirwatch.h https://invent.kde.org/frameworks/kcoreaddons/-/commit/e40eac96d2258b8b8dd8589aa3c0bbe4e8c64d05 -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 --- Comment #24 from Harald Sitter --- Git commit 65b6b7537dd3e844cb1d4ff54347de28fc34d58d by Harald Sitter. Committed on 13/11/2023 at 14:02. Pushed by sitter into branch 'kf5'. kdirwatch: always unref d, and unset d from inside d previously we could fall into a trap when the current thread for whatever reason has no local data (e.g. the dtor is invoked from the wrong thread). when that happened we would leave our d untouched and never unref, which then causes particularly hard to track down problems because we can crash at a much later point in time when the d tries to send an event to the since-deleted KDirWatch instance. instead let's revisit this a bit... the reason the `dwp_self.hasLocalData()` check was added is because of destruction order between QThreadStorage (the d) and QGlobalStatic (the ::self()) specifically because of this caveat from the QThreadStorage documentation: > QThreadStorage can be used to store data for the main() thread. QThreadStorage deletes all data set for the main() thread when QApplication is destroyed, regardless of whether or not the main() thread has actually finished. versus QGlobalStatic: > If the object is created, it will be destroyed at exit-time, similar to the C atexit function. This effectively means that QThreadStorage (at least on the main() thread) will destroy BEFORE QGlobalStatic. To bypass this problem the dwp_self check was added to conditionally skip unrefing when the QThreadStorage had already cleaned up. The trouble is that this then hides the mentioned scenario where we have a d but no backing data because of poor thread management. Instead improve our management of the pimpl: In ~KDirWatchPrivate we now unset the d pointer of all still monitoring KDirWatch. This allows us to unconditionally check for d in the ~KDirWatch so it always unrefs/removes itself from KDirWatchPrivate. In the shutdown scenario cleanup runs the other way around... KDirWatchPrivate cleans up and unsets itself as d pointer, thereby turning ~KDirWatch effectively no-op so when it runs afterwards it won't access any Private memory anymore. (cherry picked from commit 436e38ad511b44432be1be31dc4ed27383e4338a) M +11 -1src/lib/io/kdirwatch.cpp M +1-0src/lib/io/kdirwatch.h https://invent.kde.org/frameworks/kcoreaddons/-/commit/65b6b7537dd3e844cb1d4ff54347de28fc34d58d -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 --- Comment #23 from Harald Sitter --- Git commit 2fc44a3c300324c614f2d3b86fb465f2c413c70c by Harald Sitter. Committed on 03/11/2023 at 13:25. Pushed by sitter into branch 'master'. kdirwatch: don't crash after moving threads if a user moves a public watch instance across threads we used to have a d but now local data (thereby not running the dtor) and so eventually the private watch in the old thread would try to emit data into a since deleted public watch in the new thread to prevent this problem we'll "brick" the watch upon moving threads. specifically we'll fully unregister it with the old private and create/apply a new private in the new thread. since we currently have no way to reliably copy Entry as part of this we cannot really replicate the correct watches on the new thread. definitely something that could be made to work in the future though. in the meantime critically log that moving threads does not do what you expect it to do when used with kdirwatch. M +30 -0autotests/kdirwatch_unittest.cpp M +30 -0src/lib/io/kdirwatch.cpp M +5-0src/lib/io/kdirwatch.h https://invent.kde.org/frameworks/kcoreaddons/-/commit/2fc44a3c300324c614f2d3b86fb465f2c413c70c -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 Nate Graham changed: What|Removed |Added Keywords|qt6 | Version Fixed In||6.0 -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 Harald Sitter changed: What|Removed |Added Latest Commit|https://invent.kde.org/plas |https://invent.kde.org/plas |ma/kdeplasma-addons/-/commi |ma/kdeplasma-addons/-/commi |t/8245dd474d60ea34aa4aa06d3 |t/4a4439a96a44584b781e8a18b |f90b8c9922507cc |8882ab914db5f12 --- Comment #22 from Harald Sitter --- Git commit 4a4439a96a44584b781e8a18b8882ab914db5f12 by Harald Sitter. Committed on 30/10/2023 at 13:30. Pushed by sitter into branch 'Plasma/5.27'. katesessions/konsoleprofiles: do not hold profilesmodel on the stack this causes crashes when accessing the kdirwatch since it doesn't get moveToThread along with the runner. i.e. the runner thread calls into a kdirwatch that lives on the gui thread. this is particularly problematic since kdirwatch is really just a facade for thread-local backing technology, so when the wrong thread calls into kdirwatch it may tap into uninitialized memory (for that thread -- it is initialized on the "correct" thread) (cherry picked from commit 8245dd474d60ea34aa4aa06d3f90b8c9922507cc) M +2-0profiles/profilesmodel.cpp M +10 -5runners/katesessions/katesessions.cpp M +2-1runners/katesessions/katesessions.h M +10 -5runners/konsoleprofiles/konsoleprofiles.cpp M +2-1runners/konsoleprofiles/konsoleprofiles.h https://invent.kde.org/plasma/kdeplasma-addons/-/commit/4a4439a96a44584b781e8a18b8882ab914db5f12 -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 Harald Sitter changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED Latest Commit||https://invent.kde.org/plas ||ma/kdeplasma-addons/-/commi ||t/8245dd474d60ea34aa4aa06d3 ||f90b8c9922507cc --- Comment #21 from Harald Sitter --- Git commit 8245dd474d60ea34aa4aa06d3f90b8c9922507cc by Harald Sitter. Committed on 30/10/2023 at 13:27. Pushed by sitter into branch 'master'. katesessions/konsoleprofiles: do not hold profilesmodel on the stack this causes crashes when accessing the kdirwatch since it doesn't get moveToThread along with the runner. i.e. the runner thread calls into a kdirwatch that lives on the gui thread. this is particularly problematic since kdirwatch is really just a facade for thread-local backing technology, so when the wrong thread calls into kdirwatch it may tap into uninitialized memory (for that thread -- it is initialized on the "correct" thread) M +2-0profiles/profilesmodel.cpp M +10 -5runners/katesessions/katesessions.cpp M +2-1runners/katesessions/katesessions.h M +10 -5runners/konsoleprofiles/konsoleprofiles.cpp M +2-1runners/konsoleprofiles/konsoleprofiles.h https://invent.kde.org/plasma/kdeplasma-addons/-/commit/8245dd474d60ea34aa4aa06d3f90b8c9922507cc -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 --- Comment #20 from Harald Sitter --- Git commit 436e38ad511b44432be1be31dc4ed27383e4338a by Harald Sitter. Committed on 30/10/2023 at 12:27. Pushed by sitter into branch 'master'. kdirwatch: always unref d, and unset d from inside d previously we could fall into a trap when the current thread for whatever reason has no local data (e.g. the dtor is invoked from the wrong thread). when that happened we would leave our d untouched and never unref, which then causes particularly hard to track down problems because we can crash at a much later point in time when the d tries to send an event to the since-deleted KDirWatch instance. instead let's revisit this a bit... the reason the `dwp_self.hasLocalData()` check was added is because of destruction order between QThreadStorage (the d) and QGlobalStatic (the ::self()) specifically because of this caveat from the QThreadStorage documentation: > QThreadStorage can be used to store data for the main() thread. QThreadStorage deletes all data set for the main() thread when QApplication is destroyed, regardless of whether or not the main() thread has actually finished. versus QGlobalStatic: > If the object is created, it will be destroyed at exit-time, similar to the C atexit function. This effectively means that QThreadStorage (at least on the main() thread) will destroy BEFORE QGlobalStatic. To bypass this problem the dwp_self check was added to conditionally skip unrefing when the QThreadStorage had already cleaned up. The trouble is that this then hides the mentioned scenario where we have a d but no backing data because of poor thread management. Instead improve our management of the pimpl: In ~KDirWatchPrivate we now unset the d pointer of all still monitoring KDirWatch. This allows us to unconditionally check for d in the ~KDirWatch so it always unrefs/removes itself from KDirWatchPrivate. In the shutdown scenario cleanup runs the other way around... KDirWatchPrivate cleans up and unsets itself as d pointer, thereby turning ~KDirWatch effectively no-op so when it runs afterwards it won't access any Private memory anymore. M +10 -1src/lib/io/kdirwatch.cpp M +1-0src/lib/io/kdirwatch.h https://invent.kde.org/frameworks/kcoreaddons/-/commit/436e38ad511b44432be1be31dc4ed27383e4338a -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 --- Comment #19 from Bug Janitor Service --- A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kcoreaddons/-/merge_requests/386 -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 Bug Janitor Service changed: What|Removed |Added Status|CONFIRMED |ASSIGNED --- Comment #18 from Bug Janitor Service --- A possibly relevant merge request was started @ https://invent.kde.org/plasma/kdeplasma-addons/-/merge_requests/479 -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 --- Comment #17 from Bug Janitor Service --- A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kcoreaddons/-/merge_requests/385 -- You are receiving this mail because: You are watching all bug changes.
[frameworks-kcoreaddons] [Bug 472862] plasmashell and kwin_wayland randomly crashed in KDirWatchPrivate::emitEvent() due to kate sessions
https://bugs.kde.org/show_bug.cgi?id=472862 Nate Graham changed: What|Removed |Added Summary|kwin_wayland randomly |plasmashell and |crashed in |kwin_wayland randomly |KDirWatchPrivate::emitEvent |crashed in |() |KDirWatchPrivate::emitEvent ||() due to kate sessions Status|REPORTED|CONFIRMED Ever confirmed|0 |1 --- Comment #16 from Nate Graham --- Amazing detective work, as usual! I'll try disabling the "Kate Sessions" runner and see if the crashes go away. As discussed this morning, KWin crashes because it has an instance of KRunner loaded into the Overview effect. -- You are receiving this mail because: You are watching all bug changes.
