Race between editing and saving

[Kevin Ottens]

Hello,

On Sunday, 9 October 2016 10:51:28 CEST David Faure wrote:
> I've spent some more time debugging the problems when editing the
> description for a task, and losing characters. I analyzed the issue, but I
> need help choosing a solution.
> 
> Here's what happens:
> 
> 1) User types text, ArtifactEditorModel's m_text is updated.
> 2) Later on (timer-based), the model saves => Artifact::setText() + async
> job. 3) Later on (when akonadi notifies of the item having changed),
> Akonadi::Serializer::updateTaskFromItem calls Domain::Artifact::setText
> again.
> 
> One issue is that at that point trailing whitespace was removed (due to
> libical), that's easy to fix in setText with
>  if (m_text.trimmed() == text.trimmed())
>         return;
> but that only fixes the loss of whitespace, not the race described below.
> 
> What can happen is that after step 2 and before step 3, the user has time
> to type a bit more text. This change gets lost at step 3. Good news: this is
> also easy to fix, if the save timer didn't kick in again yet. Like this:
> void ArtifactEditorModel::onTextChanged(const QString &text)
> {
>     if (isSaveNeeded()) // we have pending changes, don't lose them
>         return;
> 
> But what if the save timer did kick in again already? To recap:
> 1, 2, 1b, 2b, 3
> so we have two async jobs running, and isSaveNeeded() is false again.
> When the first "item changed" notification comes in, the above fix doesn't
> help, and the text editor shows again the OLD text, losing the changes
> between 1 and 1b.

Odd how this small thing turns out difficult...
 
> I'm writing here instead of sending a patch because I don't know how to fix
> that, with the current akonadi and zanshin designs.
> 
> * EditorView not saving again until the current save job has finished?
>   (sounds very dangerous, if akonadi is stuck and we quit zanshin, or worse,
> the computer crashes, we would lose the whole set of pending changes)

Yeah, way too dangerous.

> * Ignoring notifications of changes triggered by zanshin itself? (how?)

Not something we can do, I think.

> * Ignoring outdated notifications? (how? I tried printing out revision() and
> it increases, but when saving we don't know the revision).

Same thing for the reason you mention.

> * Or even blunter: ignoring all notifications of item changes sent by
> akonadi? After all, if I *ever* edit a task in korganizer and then switch
> back to zanshin, I can very well understand that I have to switch tasks and
> back to see my change. This would be a *much* smaller problem than a text
> editor that loses characters because it's saving and reloading (via dbus
> ICP and ical serialization/deserialization) regularly.

Agreed. It's probably the best course at that point IMHO.

> * Any other idea?
> 
> PS: to reproduce the race more easily I changed
> ArtifactEditorModel::autoSaveDelay() from 500 to 50, but it also happens
> sometimes with 500, all it takes is for akonadi to be a bit busy.

Regards.
-- 
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud supporter of KDE, http://www.kdab.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/zanshin-devel/attachments/20161009/2ee23f5b/attachment.sig>