Bug 469445 - ScreenMapping entry in config file can grow infinitely due to lack of auto-removal of stale entries, which makes plasmashell slow down and eventually crash
Summary: ScreenMapping entry in config file can grow infinitely due to lack of auto-re...
Status: CONFIRMED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Folder (show other bugs)
Version: 5.27.4
Platform: Arch Linux Linux
: HI crash
Target Milestone: 1.0
Assignee: Plasma Bugs List
URL:
Keywords:
: 467529 470081 470195 471903 472277 472685 (view as bug list)
Depends on:
Blocks:
 
Reported: 2023-05-07 12:39 UTC by Nikolas Spiridakis
Modified: 2024-01-27 15:00 UTC (History)
17 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
attachment-3610397-0.html (1.29 KB, text/html)
2023-05-31 01:37 UTC, William Coolman
Details
attachment-3669542-0.html (855 bytes, text/html)
2023-05-31 14:37 UTC, William Coolman
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Spiridakis 2023-05-07 12:39:00 UTC
SUMMARY
Inside the .config/plasma-org.kde.plasma.desktop-appletsrc in the [ScreenMapping] section, the screenMapping entry for some reason keeps all the files/folders and subfolders (of all depth) ever been to the desktop. When it gets bigger, it slows down plasmashell and makes it occasionally crash. All that until it gets big enough where it makes plasmashell not being able to open (core dump).


STEPS TO REPRODUCE
1. Put a folder with many files on the desktop (I used the wine source code)
2. Look at the plasma-org.kde.plasma.desktop-appletsrc config

OBSERVED RESULT
screenMapping has all the files of that folder. As a side effects, it slows down the load of plasmashell.

EXPECTED RESULT
Is this config entry even needed? At least it should remove the files when deleted from the desktop

SOFTWARE/OS VERSIONS
Linux/KDE Plasma:
(available in About System)
KDE Plasma Version: 5.27.4
KDE Frameworks Version: 5.105.0
Qt Version: 5.15.9

ADDITIONAL INFORMATION
Comment 1 Paul Worrall 2023-05-08 08:18:48 UTC
I can confirm there's a problem with screenMapping in that file.  

I followed Nikolas's method to get a huge screenMapping entry in that file and the  next time I logged in I got "failed to allocate memory"  when I tried to launch any application using kickoff or icons pinned to the task manager.  

The system as a whole had not run out of memory as I could still launch the apps using krunner.

Removing the screenMapping entry resolved the problem.
Comment 2 Nate Graham 2023-05-22 21:40:15 UTC
*** Bug 470081 has been marked as a duplicate of this bug. ***
Comment 3 Federico Dossena 2023-05-23 04:52:48 UTC
That's exactly what I was doing, I work on wine, dxvk and vkd3d and have several folders where I build them regularly on my desktop.
Comment 4 Paul Worrall 2023-05-24 16:35:05 UTC
*** Bug 470195 has been marked as a duplicate of this bug. ***
Comment 5 William Coolman 2023-05-31 01:37:36 UTC Comment hidden (spam)
Comment 6 William Coolman 2023-05-31 14:25:25 UTC Comment hidden (spam)
Comment 7 William Coolman 2023-05-31 14:27:49 UTC
Removing [ScreenMapping] entry from /home/[user]/.config/plasma-org.kde.plasma.desktop-appletsrc fixed the problem.
Comment 8 Nikolas Spiridakis 2023-05-31 14:31:47 UTC Comment hidden (spam)
Comment 9 Federico Dossena 2023-05-31 14:32:05 UTC Comment hidden (spam)
Comment 10 William Coolman 2023-05-31 14:37:22 UTC Comment hidden (spam)
Comment 11 Paul Worrall 2023-05-31 14:41:52 UTC
*** Bug 470195 has been marked as a duplicate of this bug. ***
Comment 12 pallaswept 2023-06-28 01:56:40 UTC
TL;DR the specific nature of this issue is two-fold: that not only desktop items, but also the CONTENTS of desktop items, are stored in this file; and that files which no longer exist are never removed. 
This results in an a) oversized due to inclusion of irrelevant items, and b) ever-growing when items are removed from the desktop but not the config file; single line of text in the config file, which inevitably eventually leads to a failure to load the file and subsequent crash of plasma.


To fix this will require two parts, firstly to only track items directly ON the desktop, and NOT their contents, and secondly, to REMOVE items that are no longer present on the desktop.


>EXPECTED RESULT
> Is this config entry even needed? At least it should remove the files when deleted from the desktop

The config entry is needed to track the position of desktop items when using multiple displays.

The nature of the problem here is that it doesn't track positions of items on the desktop, it tracks positions of items on the desktop RECURSIVELY. As in, if I create a folder on my desktop, that folder needs an entry here, but the 8000 source files of the app I'm building which are within that folder do NOT need to be tracked in this file, at all, ever. They never appear on the desktop. They just happen to be stored beneath the desktop folder.

> STEPS TO REPRODUCE
> 1. Put a folder with many files on the desktop (I used the wine source code)
> 2. Look at the plasma-org.kde.plasma.desktop-appletsrc config

These steps given to cause the fault are not complete. I have a folder on my desktop right now with thousands of source files within it, but only the items I've positioned on the desktop are stored in this config file, and in this case, only the top folder is there. I discovered this bug after building the app, so something that happened during the course of building it made plasma aware of all the subdirectories and all their file contents, and created an !!8 megabyte long single line of text!! in my config file, specifying the desktop position of items that are never positioned on the desktop.

Further to this, many of those files were created during the build, and then deleted, but the entries in the config file for them remained. 

Importantly, this shows that one could cause this failure by simply creating and removing a sufficient number of files on (or within) the desktop folder. As in, eventually, it will happen to everyone.


It was only after rebooting that I discovered the problem, and thanks to Murphy's Law, the reason for rebooting was a system upgrade that included new qt libs, new plasma components, a new kernel, and a new graphics driver, so naturally I assumed one or more of those things to be the cause of the fault, and some 8 hours of troubleshooting later, I managed to pin it down to this config file. I restored from a copy from some time ago, and intended to compare it to the offending file so I didn't need to spend an hour reconfiguring my entire desktop, and Kate complained about the long line of text and then crashed, and Kdiff crashed on it too. Once I found the offending line (thank you micro text editor for being able to load it without dying), I removed it and only had to drag a few desktop icons to their rightful place. 

The experience was sufficiently unpleasant for me to create an account here and add some detail about the nature of the fault. I hope it is helpful and that this case is treated with appropriately high priority, as fixing it is no simple task. Shout out to BTRFS and snapper for saving me from a total nightmare reconfiguring everything.
Comment 13 Harald Sitter 2023-07-03 11:53:59 UTC
I expect this is a bit awkward to solve unfortunately.

We'd have to change the storage format to something more involved like

[ScreenMappingV2][DisabledScreen][0:1edf1760-75dc-4d64-9639-42be2c16fb30]
Activity=1edf1760-75dc-4d64-9639-42be2c16fb30
ScreenId=0

[ScreenMappingV2][DisabledScreen][0:1edf1760-75dc-4d64-9639-42be2c16fb30][file:///home/me/idx/2015-02-14-395.sgf]
LastSeen=2023-07-03

The currently used internal data storage inside ScreenMapper isn't really conducive for that though, it's a bunch of hash maps tracking the same data in different ways. Which is a bit nightmarish to extend. It might make for overall improved readability if we moved to a flat vector of `struct Item` instances, those could then easily carry the LastSeen data.

I'm not sure it'd necessarily improve performance but we can always add more hashing on top. There in fact a number of performance problems with ScreenMapper and FolderModel that relate to the startup speed impact that also need dealing though :(

+    struct Screen {
+        int id = -1;
+        QString activity;
+        std::optional<QUrl> url;
+    };
+
+    struct ScreenItem {
+        QUrl url;
+        bool enabled = false;
+        int lastSeen;
+        std::shared_ptr<Screen> screen;
+    };
+
+    QVector<std::shared_ptr<Screen>> m_screens;
+    QVector<std::shared_ptr<ScreenItem>> m_items;

This would then allow us to further refine the config format to include enabledness as a property

[ScreenMappingV2][0:1edf1760-75dc-4d64-9639-42be2c16fb30][file:///home/me/idx/2015-02-14-395.sgf]
Enabled=true
LastSeen=2023-07-03
Comment 14 Nikolas Spiridakis 2023-07-03 13:28:51 UTC
(In reply to Harald Sitter from comment #13)
> I expect this is a bit awkward to solve unfortunately.
> 
> We'd have to change the storage format to something more involved like
> 
> [ScreenMappingV2][DisabledScreen][0:1edf1760-75dc-4d64-9639-42be2c16fb30]
> Activity=1edf1760-75dc-4d64-9639-42be2c16fb30
> ScreenId=0
> 
> [ScreenMappingV2][DisabledScreen][0:1edf1760-75dc-4d64-9639-
> 42be2c16fb30][file:///home/me/idx/2015-02-14-395.sgf]
> LastSeen=2023-07-03
> 
> The currently used internal data storage inside ScreenMapper isn't really
> conducive for that though, it's a bunch of hash maps tracking the same data
> in different ways. Which is a bit nightmarish to extend. It might make for
> overall improved readability if we moved to a flat vector of `struct Item`
> instances, those could then easily carry the LastSeen data.
> 
> I'm not sure it'd necessarily improve performance but we can always add more
> hashing on top. There in fact a number of performance problems with
> ScreenMapper and FolderModel that relate to the startup speed impact that
> also need dealing though :(
> 
> +    struct Screen {
> +        int id = -1;
> +        QString activity;
> +        std::optional<QUrl> url;
> +    };
> +
> +    struct ScreenItem {
> +        QUrl url;
> +        bool enabled = false;
> +        int lastSeen;
> +        std::shared_ptr<Screen> screen;
> +    };
> +
> +    QVector<std::shared_ptr<Screen>> m_screens;
> +    QVector<std::shared_ptr<ScreenItem>> m_items;
> 
> This would then allow us to further refine the config format to include
> enabledness as a property
> 
> [ScreenMappingV2][0:1edf1760-75dc-4d64-9639-42be2c16fb30][file:///home/me/
> idx/2015-02-14-395.sgf]
> Enabled=true
> LastSeen=2023-07-03

I think the priority here is to make it not add subfolders/subfiles to the config. Of course improving the performance of the entry is good but remember, people's computers are crashing, so just fixing the unnecessary entries would be a good solution for now. After all the "positions" entry already handles adding/removing the correct files correctly so it seems fairly simple to me to fix the sceeenMapping..
Comment 15 pallaswept 2023-07-05 00:35:38 UTC
> I think the priority here is to make it not add subfolders/subfiles to the
> config. 

Absolutely. Like I said in my post, items which never appear on the desktop do not ever need an entry in this file, and those are the ones that are causing the greatest part of the problem, after all, there are only so many items one can actually fit on the desktop, so hitting an amount sufficient to crash plasma would be very difficult if not for the fact that it's getting subfolders.

> remember, people's computers are crashing

Again I agree strongly with this. I've just made a comment on a case (384782, about being able to re-order taskbar items) where  discussing *how* to do it has been going on for 6 years and nobody has done anything to make it *actually happen*. Time is of the essence here, let's not over-plan and under-do.

> so just fixing the unnecessary
> entries would be a good solution for now. After all the "positions" entry
> already handles adding/removing the correct files correctly so it seems
> fairly simple to me to fix the sceeenMapping..

It would be a good start to a solution, and decrease the amount of crashes experienced by more than any other part of it, for sure. But as I also mentioned in my post, the positions entry is not handling removing entries correctly. The entries that caused my crash were for files that no longer even existed.
Comment 16 Harald Sitter 2023-07-05 06:24:05 UTC
I cannot reproduce it picking up subdirectories. In any case that's just incidentally triggering the bug. If you make a folder with 1 million files your desktop it'd exhibit the same defects, which is why I've outlined a more comprehensive solution.

> so it seems fairly simple to me to fix the sceeenMapping..

patches welcome :)
Comment 17 Federico Dossena 2023-07-05 06:55:06 UTC
As a temporary workaround for people who are affected by this bug, I just run this script at login and at logout:

#!/bin/bash
cd ~
sed -i /screenMapping=/d .config/plasma-org.kde.plasma.desktop-appletsrc

It simply removes all the lines that contain "screenMapping=" from that file, it's a crappy workaround but it's better than having to reset my config every time it breaks, and it hasn't broken anything in the last couple of months that I've been using it.
Comment 18 pallaswept 2023-07-06 12:39:43 UTC
(In reply to Harald Sitter from comment #16)

> which is why I've outlined a more comprehensive solution.

A more comprehensive solution is definitely required in the long run but we don't want to wait and keep this bug active and causing crashes for the long run while we congregate to plan the best, most comprehensive solution, when an interim solution is simpler and effective. Doing both is an option, and it's a good one.

> I cannot reproduce it picking up subdirectories.

Nor can I, and yet it definitely does, because it happened to me. I can provide a backup of a file with such entries if hard proof of the behaviour is required, but *all* of the posts in this bug report ought to suffice for evidence - in fact I could provide a backup of the entire filesystem and repeat my behaviour until I can reproduce it.... but let's not get distracted. That behaviour of including subfiles/folders has been the *exclusive* cause of reported failures. It's obviously a real thing, regardless of our inability to reproduce it. The point of reproducing the cause of a bug is to find a solution, but in this case, we don't need a reproduction process to solve it. Let's not get caught up in process for the sake of process.

> In any case that's just incidentally triggering the bug. If you make a folder with 1 million files on your desktop it'd exhibit the same defects, 

You're completely correct that there are many other ways to trigger this bug. As I also mentioned in my above post, since existing entries are not removed (at least, not reliably), you could just create a single entry and remove it and repeat that process a million times. I'm sure that between us we could imagine up a few dozen at least.... 

So yes, you're completely correct that a more comprehensive solution is needed. In fact, you've highlighted a whole new conundrum, which is that there's the ability to store the desktop position of more files than can be positioned on the desktop - there's a grid size and a desktop resolution and that presents a hard limit to the number of records that should be kept, and yet, it'll presently happily store a million of them even if I can only fit a few hundred. That doesn't make any sense at all. You're right on so many levels, a more comprehensive solution is required.

But let's not ignore that the only *real* trigger for this bug so far, is the addition of subfolders/subfiles of desktop entries. That behaviour being stopped should be priority 1 because it is cause number 1 out of a total of 1, that actually triggered the bug in the real world. 

After that, we can think about how it's finding those files, why it's adding them, why it's not removing them, why it allows more than are even possible to fit, etc, etc, etc, and find a real, comprehensive solution. We can do BOTH!

> patches welcome :)

And as I also mentioned in my post above, let us beware the trap of capable, skilled, and experienced individuals (that obviously includes you) shifting responsibility to theoretical volunteers, because it's proven to result in bugs persisting for years on end while planning the best and most comprehensive solution becomes the only step taken and the problem persists indefinitely.

Meet us in the middle here mate.  I'll volunteer my time to provide as much assistance to you as possible, both to author an interim solution that prevents the addition of items beneath the desktop (as opposed to *on* the desktop), and to design and author a comprehensive solution. 

Right now, all we need to know is the function which adds items to this entry, and insert a check that the target file is actually *on, not beneath* the desktop. By the sounds of your post, you're a person with the knowledge and skill to identify that function and add the check in no time flat - it's a couple of lines of code in the right place, and by the way you're talking, it sounds like you already know the place and have the skill to write those couple of lines with ease. Please, don't waste your ability to solve a real problem with a real solution, real fast, in order to find a perfect solution for a theoretical problem, much later. You can have that cake and eat it, too.
Comment 19 Nate Graham 2023-07-25 19:09:57 UTC
*** Bug 467529 has been marked as a duplicate of this bug. ***
Comment 20 Antonio Rojas 2023-07-27 06:52:36 UTC
*** Bug 472685 has been marked as a duplicate of this bug. ***
Comment 21 xvivx 2023-07-27 21:59:37 UTC
"failed to allocate memory" 

I have the same problem. :-(

Betriebssystem: Manjaro Linux 
KDE-Plasma-Version: 5.27.6
KDE-Frameworks-Version: 5.108.0
Qt-Version: 5.15.10
Kernel-Version: 6.1.41-1-MANJARO (64-bit)
Grafik-Plattform: Wayland
Prozessoren: 12 × Intel® Core™ i7-8750H CPU @ 2.20GHz
Speicher: 15,2 GiB Arbeitsspeicher
Grafikprozessor: Mesa Intel® UHD Graphics 630     and/or  NVIDIA GeForce GTX 1050 Ti Mobile
Comment 22 xvivx 2023-07-27 22:42:19 UTC
Way to fix "failed to allocate memory" bug:

1. Edit 
/home/a/.config/plasma-org.kde.plasma.desktop-appletsrc

2. Replace "positions" line, but let YOUR resolution with:
positions={"1920x1080":[]}

3. Reboot
Comment 23 Nate Graham 2023-07-29 14:11:57 UTC
Getting more reports of this on discuss.kde.org; raising priority.
Comment 24 Bug Janitor Service 2023-08-15 21:43:29 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-desktop/-/merge_requests/1664
Comment 25 Harald Sitter 2023-08-18 14:35:30 UTC
Git commit 8c893445732c76e30ec0cd72aab2036b30078c2a by Harald Sitter.
Committed on 18/08/2023 at 16:34.
Pushed by sitter into branch 'master'.

folderview: cap amount of screen mappings we hold

this introduces a hard limit for how many items we support on the
desktop. currently that is a static limit so it's not too invasive for
5.27. ideally we should revisit this for 6 and try to deal with
excessive items more gracefully

M  +11   -0    containments/desktop/package/contents/ui/FolderView.qml
M  +6    -0    containments/desktop/plugins/folder/CMakeLists.txt
M  +46   -3    containments/desktop/plugins/folder/screenmapper.cpp

https://invent.kde.org/plasma/plasma-desktop/-/commit/8c893445732c76e30ec0cd72aab2036b30078c2a
Comment 26 Nikolas Spiridakis 2023-08-18 15:08:07 UTC
I don't want to be rude but this does not fix the core problem. I mean plasma shell doesn't crash anymore but after you put a folder with a lot of files it stops logging screen mapping entries. Realistically no one should have so many monitors to cap the limit. That is, if the entries are legit (aka not the subfiles/folders which as far as I understand don't serve a purpose there).
Comment 27 Oliver Beard 2023-08-18 16:05:46 UTC
(In reply to Nikolas Spiridakis from comment #26)
> I don't want to be rude but this does not fix the core problem.

The goal is to resolve the slowdown and crashing in 5.27. It's quite likely that the core issue will be addressed for Plasma 6, but such major work would not be brought to 5.27.
Comment 28 pallaswept 2023-08-23 16:43:02 UTC
(In reply to Oliver Beard from comment #27)
> (In reply to Nikolas Spiridakis from comment #26)
> > I don't want to be rude but this does not fix the core problem.
> 
> The goal is to resolve the slowdown and crashing in 5.27. It's quite likely
> that the core issue will be addressed for Plasma 6, but such major work
> would not be brought to 5.27.

That seems very sensible to me.... but if that is the intention then this case should not be marked as resolved and fixed until it is actually resolved and fixed, which it isn't.  Perhaps it would be preferable to create a new bug report for work on Plasma 6 and resolve this case with reference to that new bug. But leaving a bug existing with knowledge of it and no bug open basically means "we hope someone remembers to get back to this or we're going to find out when it comes back and bites us in the butt later on". Please don't do stuff that's going to create more problems in the future. I don't want it to come back and bite you or I later on. We all want the KDE developers' lives to be easier, and this is not the means to that end.

I'll leave it to somebody in a position of authority to reopen this case or create a new one or whatever means best ensures that this isn't forgotten.

Anyway enough discussion of the correct process for resolving cases. I'll take off my ITIL hat and put my end-user hat back on now.

Thank you very much for providing us all with this workaround in the meantime, KDE is amazing with thanks to people like you and I miss it when it doesn't load :) Now I can close my config file I've been nervously checking every day, and relax, and it's thanks to you, Olivier, Nate, especially Harald for writing the code nice one mate!.... and anybody else I might have missed. Greatly appreciated! <3
Comment 29 Harald Sitter 2023-08-28 22:16:24 UTC
Git commit 9ed48f1def75f3ce46853ddb212b7cb7247e3a7c by Harald Sitter.
Committed on 29/08/2023 at 00:12.
Pushed by sitter into branch 'Plasma/5.27'.

folderview: cap amount of screen mappings we hold

this introduces a hard limit for how many items we support on the
desktop. currently that is a static limit so it's not too invasive for
5.27. ideally we should revisit this for 6 and try to deal with
excessive items more gracefully


(cherry picked from commit 8c893445732c76e30ec0cd72aab2036b30078c2a)

M  +11   -0    containments/desktop/package/contents/ui/FolderView.qml
M  +6    -0    containments/desktop/plugins/folder/CMakeLists.txt
M  +46   -3    containments/desktop/plugins/folder/screenmapper.cpp

https://invent.kde.org/plasma/plasma-desktop/-/commit/9ed48f1def75f3ce46853ddb212b7cb7247e3a7c
Comment 30 Nate Graham 2023-09-11 22:20:23 UTC
*** Bug 472277 has been marked as a duplicate of this bug. ***
Comment 31 Nate Graham 2023-09-13 18:46:09 UTC
*** Bug 471903 has been marked as a duplicate of this bug. ***
Comment 32 Nate Graham 2023-09-30 04:15:12 UTC
For those of you who were concerned that we were just papering over the actual bug here, you were right (though it was intentional as a quick fix), and will hopefully be happy to learn that the actual root cause has been identified and fixed with https://invent.kde.org/plasma/plasma-desktop/-/commit/685fac5355812cfce67c4008c38fa3fbf5f75b08. Ultimately Harald and Marco figured it out and fixed it for good. Even better, that fix will be in Plasma 5.27.9!
Comment 33 pallaswept 2023-10-10 05:24:35 UTC
(In reply to Nate Graham from comment #32)
> For those of you who were concerned that we were just papering over the
> actual bug here, you were right (though it was intentional as a quick fix),
> and will hopefully be happy to learn that the actual root cause has been
> identified and fixed with
> https://invent.kde.org/plasma/plasma-desktop/-/commit/
> 685fac5355812cfce67c4008c38fa3fbf5f75b08. Ultimately Harald and Marco
> figured it out and fixed it for good. Even better, that fix will be in
> Plasma 5.27.9!

TL;DR you did it the right way, but I think maybe(?) you missed a spot with the patch.

Fantastic news. I wouldn't say I felt that you were 'papering over' it, in fact it was me who initially suggested (see comment 18) that a quick fix be put in place before a more complete fix was done over time. You've done exactly that, so I'm very pleased. It's not entirely clear to me though, but there may still be one outstanding issue, please refer to comment 12  - the fault here was in two parts; in brief, first that files were added recursively (which is obviously fixed by this patch) and second, that non-existent items were not removed. I'm not so sure that second part has been approached by this patch? Please forgive me, I'm not sufficiently familiar with the codebase to be able to be certain by looking at the patch, whether the cleanup is taken care of by that new file and it's two functions, or whether they are only used to filter additions to non-recursive entries.... but at a glance, it appears tha the former is the case, and the removal of non-existent entries has not been resolved by this patch.
Comment 34 pallaswept 2023-10-10 05:39:27 UTC
To be clear, I do not mean to complain - as I said I'm very happy and I think you've taken the right approach and achieved a good result (both in the short and longer term). It's just that I also know that Harald was enthusiastic about putting in a real, proper fix (to quote him, a "comprehensive solution"), and while I suggested a short term quick-fix beforehand, I always agreed with this need for a comprehensive solution, and I think he might want to consider that cleanup as a part of his admirably thorough approach.

I won't complain if you decide not to bother with this second part - in reality, it would be far more difficult now (without recursive additions) to trigger the fault. You'd need to do something pretty weird, like, add items to the KDE desktop, leave KDE, remove the files (in some other DE or just a shell) and then repeat that a few times, to make the list grow, until it crashed - and that seems a very unlikely use-case. But I'm sure some unfortunate person will find that trip-hazard one day, if it's left there. Probably 5 years from now when everyone's forgotten about this bug report XD But I do think that it'd best to be taken care of it, and I know Harald wants to be thorough and comprehensive, so that's why I mention it. I'll leave it in your capable hands.
Comment 35 Nate Graham 2023-10-10 15:37:01 UTC
Non-existent items get removed for me. Or at least, when I delete a file or a folder using the desktop, its entry gets removed from the config file.

Deleting items from elsewhere also works for me; e.g. if I `rm` a file on the desktop using a terminal, Plasma notices it and removes its config file entry. Which means in general this feature works, and any issues with it are edge case bugs. If you have a reproducible way to make that happen, it would be good to submit a new bug report that indicates how you can reproduce it.
Comment 36 pallaswept 2023-10-16 02:26:05 UTC
(In reply to Nate Graham from comment #35)
> Non-existent items get removed for me.

TL;DR: Try harder, because it's a thing.

Remember that my original reason for being here was that I had built a software package on the desktop, and the files had since been deleted, but their entries were still there in the list in the config file. That was what caused the crash that brought me here. Like I said, I ha ve backups, I could restore them and spend a week trying to figure out exactly what I did, but let's be reasonable here. I didn't come here just for your good looks :) The failure to remove deleted files from the list in the config file is literally why I'm here. If it had added all the subdirectories (the thing that's just been patched) and then cleaned up after itself, there'd have been 4 entries in that list in the config file and the crash would have never happened.

Certainly, deleting them in certain ways causes them to be removed
Certainly, deleting them in other ways causes them not to be removed

If you can't reproduce the failure to remove files and have them remain in the list in the config file, try harder :D Because it's a thing. It was in my first post for a reason.

Just as it was for the file being added to the list in the first place - remember all the people who copied lots of files to the desktop and didn't get any entries, and said "hey I can't reproduce this" (including the man who write the fix we have now that fixes exactly what he couldn't reproduce!), but meanwhile all the reports of people with source code that was being added as subdirectories? This is that, in reverse.

Sometimes you add the files and KDE knows about it and adds them to the list, sometimes you add them in some other way and it doesn't add them to the list.

Sometimes you remove the files and KDE knows about it and removes them from the list, sometimes you remove them in some other way and it doesn't remove them from the list.

>  If you have a reproducible way to make that happen, it would be good to submit a new bug report that indicates how you can reproduce it.

Are we really going to start all over with the "I can't reproduce it, it isn't a thing", thing? It's a thing. I swear. If it weren't, I would never have even had the crash that led me to create this account to make that post that started off by explaining that it's the 2nd half of the problem.



> in general this feature works, and any issues with it are edge case bugs.

The  feature that *does* work, is that when KDE sees it get removed, it removes it from the list. The feature that *doesn't exist*, is checking that entries which are in the list, have not been removed. 

There's no check to make sure it didn't miss something. That's why, when my software I built from a directory on the desktop, created thousands of files (downloading python and rust dependencies for the package I was building), and subsequently deleted them as part of the build script, and KDE didn't notice, those entries stayed in the list, and my PC crashed the next time I rebooted, and that's why I'm here.

It's a thing. Reproducing it might be tricky, I get it.... but let's not pretend it's not a thing, and that there isn't a 'trap' there, where if, for some inexplicable reason, KDE fails to remove a non-existent file from the list, **it will never check to make sure it removed everything it should have.**
Comment 37 pallaswept 2023-10-16 03:18:46 UTC
Here I can prove it easier than I thought.

My intention was to create a few files on the desktop, have them added to the screenmapping entry, then try a few different ways to remove them until I found one that made it remain in the screenmapping entry. So I did this, making these 3 'foo' files on the desktop:

~>echo foo1 > ~/Desktop/foo1.txt
 ~>echo foo1 > ~/Desktop/foo2.txt
 ~>echo foo1 > ~/Desktop/foo3.txt

Then I thought, I'll check the screenmapping and make sure they've been added, before I try to remove them and have them remain in that list

 ~>cat  .config/plasma-org.kde.plasma.desktop-appletsrc | rg screenmapping
[ScreenMapping]
screenMapping=desktop:/userworking/default.target.wants/ydotool.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/default.target.wants,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/Media,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/dbus-org.bluez.obex.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/pipewire-pulse.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/pwdumpxbox.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/tmuxkeys2.conf.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/pipewire.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/wireplumber.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/foo1.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/tmuxkeys3.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/foo2.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/foo3.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/OpenRGB.desktop,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/tmuxkeys.conf.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/jctlpw.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/pipewire.socket,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,file:///home/pallaswept/Desktop/Mouse-actions GUI.desktop,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/pwdumpalways2.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef

WTF. Half of those files don't even exist already. Proof:

 ~>ls ~/Desktop
total 119M
   0 drwxr-xr-x  1 pallaswept pallaswept  266 Oct 16 14:07 ./
   0 drwx--x---+ 1 pallaswept pallaswept 1.2K Oct 16 12:58 ../
4.0K -rw-r--r--  1 pallaswept pallaswept    5 Oct 16 14:07 foo1.txt
4.0K -rw-r--r--  1 pallaswept pallaswept    5 Oct 16 14:07 foo2.txt
4.0K -rw-r--r--  1 pallaswept pallaswept    5 Oct 16 14:07 foo3.txt
115M -rw-r--r--  1 pallaswept pallaswept 115M Oct  1 19:22 jctlpw.txt
   0 drwxr-xr-x  1 pallaswept pallaswept  478 Sep 10 02:50 Media/
4.0K -rwx------  1 pallaswept pallaswept  305 Aug 25 02:30 OpenRGB.desktop*
1.6M -rw-r--r--  1 pallaswept pallaswept 1.6M Oct  5 17:49 pwdumpalways2.txt
1.6M -rw-r--r--  1 pallaswept pallaswept 1.6M Oct 16 06:15 pwdumpxbox.txt
 28K -rw-r--r--  1 pallaswept pallaswept  27K Oct  4 21:36 tmuxkeys2.conf.txt
 32K -rw-r--r--  1 pallaswept pallaswept  30K Oct  6 05:44 tmuxkeys3.txt
 32K -rw-r--r--  1 pallaswept pallaswept  29K Oct  2 15:29 tmuxkeys.conf.txt

Notice all the entries in the screenmapping that don't even exist in the file listing? btw I alias ls to 'ls -Flash', so no, they aren't hidden or something. They're files I deleted ages ago.
Comment 38 pallaswept 2023-10-16 03:23:25 UTC
Just to be more precise, here's my alias so that you can be sure my 'ls' listing that shows all the files in the directory (none are hidden) and also shows that the list contains all the files that do exist, and a bunch of files that don't exist:

alias ls='ls -shalF --color'
Comment 39 pallaswept 2023-10-16 03:40:22 UTC
More specifics to show that the entries in my screenmapping are nonexistent files:

ls ~/Desktop/userworking
ls: cannot access '/home/pallaswept/Desktop/userworking': No such file or directory <--- Isn't this there and full of backups of systemd unit files?
find ~/Desktop/ -name *.service                                                                                                                                                                                2 
find ~/Desktop/ -name *.socket
find ~/Desktop/ -name *.desktop
/home/pallaswept/Desktop/OpenRGB.desktop
<---  where is  Mouse-actions GUI.desktop?

IT IS A THING I SWEAR.

So, to fix this, you have two ways that can work: 
1) Find every possible way out of an undefined number of possible ways, to remove a file, and have it left in the list, and fix every one of them (ie hey, pallaswept, I can't reproduce it, so you reproduce it, N times for the N variations that cause this (N being an undefined number that could be infinite for all we know), and log a new case N times)
OR
Just check that the entries in the list actually exist and remove them if they don't.

Tell me which one is more sane.
Comment 40 pallaswept 2023-10-16 03:59:03 UTC
There, I reproduced it.

Create the three foo*.txt files exactly as above
Log out
Log into IceWM session
Open xterm
Delete foo*.txt files

Result:

cat  .config/plasma-org.kde.plasma.desktop-appletsrc | rg screenmapping
[ScreenMapping]
screenMapping=desktop:/tmuxkeys3.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/pwdumpalways2.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/pipewire.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/foo1.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/tmuxkeys2.conf.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/pwdumpxbox.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/default.target.wants/ydotool.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/wireplumber.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/foo2.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/jctlpw.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/pipewire-pulse.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/tmuxkeys.conf.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,file:///home/pallaswept/Desktop/Mouse-actions GUI.desktop,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/OpenRGB.desktop,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/default.target.wants,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/dbus-org.bluez.obex.service,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/userworking/pipewire.socket,1,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/Media,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef,desktop:/foo3.txt,0,daba52e7-d61d-46e5-a570-bf4e3a1014ef
 ~ls Desktop/
total 119M
   0 drwxr-xr-x  1 pallaswept pallaswept  218 Oct 16 14:52 ./
   0 drwx--x---+ 1 pallaswept pallaswept 1.2K Oct 16 14:53 ../
115M -rw-r--r--  1 pallaswept pallaswept 115M Oct  1 19:22 jctlpw.txt
   0 drwxr-xr-x  1 pallaswept pallaswept  478 Sep 10 02:50 Media/
4.0K -rwx------  1 pallaswept pallaswept  305 Aug 25 02:30 OpenRGB.desktop*
1.6M -rw-r--r--  1 pallaswept pallaswept 1.6M Oct  5 17:49 pwdumpalways2.txt
1.6M -rw-r--r--  1 pallaswept pallaswept 1.6M Oct 16 06:15 pwdumpxbox.txt
 28K -rw-r--r--  1 pallaswept pallaswept  27K Oct  4 21:36 tmuxkeys2.conf.txt
 32K -rw-r--r--  1 pallaswept pallaswept  30K Oct  6 05:44 tmuxkeys3.txt
 32K -rw-r--r--  1 pallaswept pallaswept  29K Oct  2 15:29 tmuxkeys.conf.txt

Note the foo*.txt file still present in the screenmapping section but no longer exist since I just deleted them.

First I tried just dropping to a TTY by hiting ctrl+alt+f5, logging in as a different user (root because why not) and deleting them when KDE was still running, and then I ctrl+alt F1 back to my running KDE session and it had instantly removed them from the screenmappinges entry. So I figured, let's not do it with KDE running, and see how it fares. Now we know.
Comment 41 pallaswept 2023-10-16 04:06:47 UTC
Oh and BTW that means that 'N' was infinite because all we have to do to reproduce this is not be running KDE when we remove the files and there are practically infinite ways to go about that. Literally every way but one (be running KDE). So you can see why I wasn't satisfied with my first and now proven correct description of the nature of the fault being ignored and told to go log a new case.

I think I've made my point sufficiently, yeh?
Comment 42 pallaswept 2023-10-16 04:22:02 UTC
(In reply to pallaswept from comment #41)
>  Literally every way but one (be running KDE). 

I just said this but then I realised there's not even that one way - let's not forget, this:

> Remember that my original reason for being here was that I had built a software package on the desktop, and the files had since been deleted, but their entries were still there in the list in the config file.

.... happened in a KDE session (I'm ALWAYS in a  KDE session, it's the best, why would I not be? :) )

So it'll *always* do it if KDE isn't running, and *sometimes even when it is*. The point is there's a theoretically insanely high number of ways to trigger this and it doesn't matter what they are if you just, to quote the second sentence of my first post:

> To fix this will require two parts, firstly to only track items directly ON the desktop, and NOT their contents,

Which Harald has now done, yay!

> and secondly, to REMOVE items that are no longer present on the desktop.

Which is why I said it was important to get this second part done, to be, as Harald put it, "comprehensive" in the solution.
Comment 43 Nate Graham 2023-10-20 18:11:11 UTC
(In reply to pallaswept from comment #39)
> So, to fix this, you have two ways that can work: 
> 1) Find every possible way out of an undefined number of possible ways, to
> remove a file, and have it left in the list, and fix every one of them (ie
> hey, pallaswept, I can't reproduce it, so you reproduce it, N times for the
> N variations that cause this (N being an undefined number that could be
> infinite for all we know), and log a new case N times)
> 
> OR
> 
> Just check that the entries in the list actually exist and remove them if
> they don't.
That's the crux of it.

The issue with option #1 is that we'll be playing whack-a-mole forever, and it's not even theoretically fixable for the case you provided where you log into a non-Plasma desktop, delete some files, and lot into Plasma again.

This leaves option #2, which is pretty easy but the problem is that checking for the existence of all those files requires a bunch of stat calls. If for example we did them at plasmashell startup in a blocking fashion, it would slow down plasmashell's startup or make it hang if you've done something silly like put a symlink on the desktop that points to a file on an unmounted NFS share (yes, people do crazy things like this).

Maybe it would work if we made the stat calls non-blocking and did them in the background in a different thread, rather than all at once on startup, it would be fine.

Rer-opening and lowering priority, since it's now been partially fixed enough to make this not be a pants-on-fire level issue.
Comment 44 pallaswept 2023-10-21 11:20:19 UTC
(In reply to Nate Graham from comment #43)
> The issue with option #1 is that we'll be playing whack-a-mole forever,

Exactly.

> This leaves option #2, which is pretty easy but the problem is that checking
> for the existence of all those files requires a bunch of stat calls. If for
> example we did them at plasmashell startup in a blocking fashion, it would
> slow down plasmashell's startup or make it hang .....

Yeh that would suck. It's a good thing we have people like you who understand how KDE works so that you can see these problems coming before they happen. Thank you!
I do wonder though, if you'd actually need to make so many stat calls. You could approach this by reading each screenmapping entry, and making a stat call to confirm its existence  - or, a single listing of the desktop directory's contents could be taken, and then it's an easy check that each screenmapping entry item is in that listing.
Any way, I'm sure someone who knows better than me will figure out the best way to do it.

> Maybe it would work if we made the stat calls non-blocking and did them in
> the background in a different thread, rather than all at once on startup, it
> would be fine.

I think that should be fine. I'd say it can take as long as it needs, so long as it reads the screenmapping entry, does its cleanup, and completes before reading the screenmapping entry and applying it; or potentially, reading the screenmapping entry and crashing because it's too long. 

But I guess that leaves a situation where one might log into a plasma desktop session, and have un-arranged icons for a moment, before the cleanup thread finishes and allows the implementation of the screenmapping entry? Not really a big drama, but just food for thought. Delayed arrangement of icons is certainly better than delaying the whole startup like you described! (And, it would only happen just that once, when there was something to clean up - the next startup would be like normal)

> Rer-opening and lowering priority, since it's now been partially fixed enough to make this not be a pants-on-fire level issue.

Legend, thank you!
FWIW I agree, it's not super urgent now, with the patches Harald made, the user would have to do something fairly out of the ordinary,  to make it fail.
Comment 45 pallaswept 2023-10-23 11:39:05 UTC
Quick helper comment for end-users suffering/fixing this issue:

Today, I tried to clean up my screenmapping entry, and remove the entries (seen above) for files that didn't exist. Since I only need two positioned, I just removed all off them (so, screenmapping=<no text here>) , then saving the file. Then I just moved the two remaining desktop items, and it would recreate the screenmapping  entry - complete with the glitched entries for files that didn't exist! I guess it wa cached somewhere.

So, I cleaned up the screenmapping entry by completely emptying it (so, screenmapping=<no text here>) and saved the file, and didn't move any icons (so that that screenmapping entry would not be updated), and rebooted (just log out/log in would probably work just the same)

When I logged in, the plasma session kinda locked up. I saw my desktop, I could move my mouse, and it was leaving a trail of cursors, and the panels didn't appear. It was just like when the screenmapping entry was too long. "Oh no, I broke it!", I thought. It was unresponsive like this, with the mouse cursor leaving a trail and an empty desktop, for a good 20 seconds. I was thinking I might have to kill the session somehow and restore the glitched screenmapping entry.......... And then suddenly, it all came back to life, the panels appeared, the mouse stopped leaving a trail, and I moved my two icons to where I want them, and the appropriate screenmapping entries were created, without all the extra junk, and everything is fine.

So TL;DR, if you empty the screenmapping entry, then reboot, and when you log in again it seems like it's crashed/frozen.....be patient, Really patient, I'm not exaggerating, it was a good 20+ seconds of unresponsive UI+glitched mouse cursor+no panels..... It seemed like forever and I was about to give up and kill it..... but it DID come back in the end. Just give it a little more time than you think it needs, it'll probably be OK any minute now....
Comment 46 fanzhuyifan 2024-01-27 07:36:11 UTC
I can't seem to reproduce on plasma 6 RC1. Is this still a problem?
Comment 47 Federico Dossena 2024-01-27 08:05:27 UTC
No, this was fixed a while ago as far as I can tell.

On 1/27/24 08:36, bugzilla_noreply@kde.org wrote:
> https://bugs.kde.org/show_bug.cgi?id=469445
>
> fanzhuyifan@gmail.com changed:
>
>             What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                   CC|                            |fanzhuyifan@gmail.com
>
> --- Comment #46 from fanzhuyifan@gmail.com ---
> I can't seem to reproduce on plasma 6 RC1. Is this still a problem?
>
Comment 48 Nate Graham 2024-01-27 15:00:38 UTC
The issue is effectively fixed now for 99% of real-world use cases. The bug report is still open (at a reduced severity level) due to that remaining 1% possibility with certain unusual use cases.