<div>sebas created this revision.<br />
sebas added a reviewer: graesslin.<br />
sebas added a subscriber: Plasma.<br />
Restricted Application added a project: Plasma.<br />
Restricted Application added a subscriber: plasma-devel.</div><br /><div><strong>REVISION SUMMARY</strong><div><p>Use a timer to avoid catching configChanged signals after we set<br />
changes.</p>

<p>The long version:</p>

<p>TL;DR: We have a race condition when the kscreen daemon starts. It looks<br />
up a known config, then applies it and subsequently resaves the config.<br />
The same happens on config changes, it writes, then re-reads and then<br />
re-writes the config change.<br />
I've managed to prevent this from happening by adding a timer that does<br />
avoids saving the config as a direct reaction to our own config changes.</p>

<p>So what happens on kded5 startup after loading the kscreen2 module:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">the kscreen config is requested and received</li>
<li class="remarkup-list-item">the kscreen daemon (KD) looks into its config directory for a suitable config file (a config file is identified by a combined hash of all screen</li>
</ul>

<p>attached, so unique per connected set of outputs)</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">KD usually finds a config</li>
<li class="remarkup-list-item">KD ignores configChanged events before it starts ...</li>
<li class="remarkup-list-item">a KScreen::SetConfigOperation to apply the "known config"</li>
<li class="remarkup-list-item">SetConfigOperation returns after a while (say 100ms later)</li>
<li class="remarkup-list-item">we re-enable the change monitor</li>
<li class="remarkup-list-item">we receive a configChanged signal</li>
<li class="remarkup-list-item">we save the new config (usually to the existing config file)</li>
</ul>

<p>I don't think this behavior is desirable. I don't see a reason why the<br />
daemon should save its config right after applying it. I think this<br />
causes more problems than we want, since the startup may overwrite the<br />
user's config. This behavior seems to be desired by the code in KD, it's<br />
already blocking configChanged signals during the SetOperation (which,<br />
to be honest may result in nightmarish behavior in any way, so it might<br />
be a kludge which aims too short).</p>

<p>From libkscreen perspective, SetConfigOperation::finished cannot<br />
guarantee that all configChanged signals are already fired and that it's<br />
safe to watch for new, independent changes now. At least on X11, we<br />
simply don't know, and what we can do is wait a bit and cross fingers<br />
that we're not catching our own noise. The changed signal *may* come<br />
from a re-request of the edid information, but this is a bit hard to<br />
track down, and not too useful, anyway, since changed Edid may affect a<br />
large number of a screen's properties.<br />
In the Wayland backend, that's a different story and we can prevent this<br />
behavior at an earlier stage, so this timer is "probably not needed" (I<br />
haven't tested that).</p>

<p>This effectively prevents KD from catching reactions to its own changes<br />
and does not trigger saving the config file on every login. It still<br />
reacts to changes from libkscreen, but will avoid re-saving the config a<br />
few times. The timer may not be the neatest of solutions for this, but<br />
it does help narrowing down the problem and may be a last resort action.<br />
Most importantly, it avoids the re-writing of the config on startup and<br />
plugging/unplugging a monitor effectively.</p>

<p>The timer value of 100ms is also used in kwin, which should make the<br />
behavior (which is no problem in kwin) more solid.</p>

<p>CCBUG:346961<br />
CCBUG:358011</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>rKSCREEN KScreen</div></div></div><br /><div><strong>BRANCH</strong><div><div>sebas/setop-race</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D1730" rel="noreferrer">https://phabricator.kde.org/D1730</a></div></div><br /><div><strong>AFFECTED FILES</strong><div><div>kded/daemon.cpp<br />
kded/daemon.h</div></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>sebas, graesslin<br /><strong>Cc: </strong>plasma-devel, Plasma, sebas<br /></div>