Bug 448778 - Unplugging an external monitor leads to a crash
Summary: Unplugging an external monitor leads to a crash
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Unclassified
Component: platform-drm (show other bugs)
Version: git-stable-Plasma/5.24
Platform: Archlinux Packages Linux
: NOR crash with 1 vote (vote)
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords: wayland
Depends on:
Blocks:
 
Reported: 2022-01-19 15:37 UTC by Matej Mrenica
Modified: 2022-01-28 00:45 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.24


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matej Mrenica 2022-01-19 15:37:28 UTC
SUMMARY

STEPS TO REPRODUCE
1. Log into Plasma (Wayland)
2. Switch screens to "external monitor only"
3. unplug the monitor

OBSERVED RESULT
Kwin crashes

EXPECTED RESULT
It doesn't crash

SOFTWARE/OS VERSIONS
Kwin from git: Plasma 5.24 + https://invent.kde.org/plasma/kwin/-/merge_requests/1908

Log: https://pastebin.com/n35DGgM5
Comment 1 Nate Graham 2022-01-19 15:52:51 UTC
Does this happen when *not* running with that merge request? If not, can you comment in that merge request instead of here?
Comment 2 Matej Mrenica 2022-01-19 16:11:20 UTC
(In reply to Nate Graham from comment #1)
> Does this happen when *not* running with that merge request? If not, can you
> comment in that merge request instead of here?

Yes: https://pastebin.com/nns7Jn5b I could comment there but I was told to create a new report.
Comment 3 Zamundaaa 2022-01-19 16:28:25 UTC
I added a bunch of debugging prints to the MR. Can you trigger the crash with that and then attach the logs for kwin_wayland_drm here?
You can get them with either
$ grep kwin_wayland_drm ~/.local/share/sddm/wayland-session.log
or
$ journalctl --boot 0 | grep kwin_wayland_drm
(which depends on your distro)
Comment 4 Matej Mrenica 2022-01-19 17:52:53 UTC
I must be doing something wrong because neither of those commands produce any output.
What I did:
I cloned kwin branch Plasma/5.24 and applied your patch.
Then I built and installed kwin with: "cmake -B build  -DCMAKE_INSTALL_LIBEXECDIR=lib -DBUILD_TESTING=OFF".
Then I set QT_LOGGING_RULES="kwin_wayland_drm.debug=true;kwin_core.debug=true;kwin_wayland_backend.debug=true" in /etc/environment
Finally, I re-logged in and triggered the crash.
Comment 5 Zamundaaa 2022-01-19 19:17:43 UTC
Maybe logging in the legacy session is still broken... Try 
$ kwriteconfig5 --file startkderc --group General --key systemdBoot true
and reboot.
Comment 6 Matej Mrenica 2022-01-20 07:59:25 UTC
OK, now it gives an output: https://pastebin.com/Jb6T2m5F
Comment 7 Zamundaaa 2022-01-20 08:24:25 UTC
Did you reproduce the crash before getting the log? It only shows the external output being connected, not removed. Up to that point everything looks correct
Comment 8 Matej Mrenica 2022-01-20 08:42:35 UTC
In the log I provided I unplug the cable before 08:52:41, is that what you are asking? After that only a cursor appears on my laptop screen and after a few seconds Plasma is restarted (only with systemd startup).
Comment 9 Zamundaaa 2022-01-21 00:21:25 UTC
Sorry, didn't scroll up enough. KWin reads the configuration from KScreen, it is correct (eDP-1 is supposed to be enabled again) and then it gets applied... But it silently fails, which it really should not. 
I could now reproduce it myself though; at least on my laptop KScreen somehow uses a different refresh rate number (60001) than KWin (60000) and that makes applying the changes fail.

I didn't find the underlying bug yet but I pushed a commit to the MR that makes KWin not depend on the refresh rate to match in order to not crash.
Comment 10 Matej Mrenica 2022-01-21 08:20:03 UTC
Latest version of the MR fixes this issue. Unplugging the cable doesn't make Kwin crash anymore.
Comment 11 Zamundaaa 2022-01-27 18:57:13 UTC
Git commit 74b1fb0ab5344d3189e77538fc03c60c45b02707 by Xaver Hugl.
Committed on 27/01/2022 at 18:40.
Pushed by zamundaaa into branch 'master'.

backends/drm: round refresh rate values from KScreen configs

The implicit cast effectively rounds the value down, which make the
refresh rate be different from what KScreen actually wrote.
A better fix would be to use integers instead of floating point numbers
but that needs to happen in KScreen.

M  +2    -1    src/backends/drm/drm_backend.cpp

https://invent.kde.org/plasma/kwin/commit/74b1fb0ab5344d3189e77538fc03c60c45b02707
Comment 12 Zamundaaa 2022-01-27 19:13:13 UTC
Git commit 9f9ad7036518f0d0df984a3eda28e2b4c57607d3 by Xaver Hugl.
Committed on 27/01/2022 at 19:13.
Pushed by zamundaaa into branch 'Plasma/5.24'.

backends/drm: round refresh rate values from KScreen configs

The implicit cast effectively rounds the value down, which make the
refresh rate be different from what KScreen actually wrote.
A better fix would be to use integers instead of floating point numbers
but that needs to happen in KScreen.


(cherry picked from commit 74b1fb0ab5344d3189e77538fc03c60c45b02707)

M  +2    -1    src/backends/drm/drm_backend.cpp

https://invent.kde.org/plasma/kwin/commit/9f9ad7036518f0d0df984a3eda28e2b4c57607d3