Krita Saving Issues
Open, Needs TriagePublic

Description

This task was created based on a Google Drive document.

It is about saving issues in Krita, particularly saving without an output file.

“I saved, but there is no file”
(Krita should tell the user that something bad happened or not mark the document as saved)
https://www.reddit.com/r/krita/comments/bgpj6x/cant_find_my_saved_file/
https://www.reddit.com/r/krita/comments/bejj8j/cant_find_my_file_ive_saved/
https://www.reddit.com/r/krita/comments/ak188i/krita_doesnt_save_progress_even_when_i_have_click/
https://www.reddit.com/r/krita/comments/aj7ijf/krita_cant_save_my_art/
https://www.reddit.com/r/krita/comments/9zudfp/krita_did_not_save_my_document_on_mac/
https://forum.kde.org/viewtopic.php?f=139&t=160457
https://forum.kde.org/viewtopic.php?f=139&t=160454

  • Here some solved ones:

(saving to OneDrive)
https://www.reddit.com/r/krita/comments/bod4p9/krita_document_cannot_save/
(because of flatpak?)
https://www.reddit.com/r/krita/comments/byfzmb/cant_save_any_work_no_message_appears/

Repeated pattern:
“I for sure saved, but the file is not there”

harder to make sure they actually saved…
sometimes it can be a problem with OneDrive/Google drive etc.
No way to reproduce reliably?

Suspected reason: OneDrive/dropbox or its workaround

Related:
https://bugreports.qt.io/browse/QTBUG-57299
https://bugs.kde.org/show_bug.cgi?id=394376#c13
https://phabricator.kde.org/P235
https://phabricator.kde.org/D15184
https://code.woboq.org/qt5/qtbase/src/corelib/io/qfilesystemengine_unix.cpp.html#_ZN17QFileSystemEngine8copyFileERK16QFileSystemEntryS2_R12QSystemError

tymond created this task.Jul 8 2019, 10:40 AM

I managed to reproduce it that way:
Open a big animation file.
Add a new layer on top of everything.
Add a new frame on 0th.
Draw something on it.
Wait for autosave to kick in and finish its work. (Save during autosave most probably won’t work, either, but it will leave the file unsaved, so Krita will ask whether we want to close it or not)
Save the file.
Very quickly click “X” on the window top bar; Windows ask if it should close Krita; don’t do anything
Krita will start responding and it will close just after that.
No saved file anywhere.
Not sure how reproduceable it is in general. Also there is that additional thing that Windows ask what to do with unresponding Krita - no one reported it beforehand.
I guess it happens when Krita is busy for example making a copy of the image and maybe that happens before Krita starts blocking itself because of the saving going on…

If you miss that “unresponsiveness” gap, Krita will tell you that saving is in progress and if that happens, the file is properly saved.

I could provide the file that makes it possible to reproduce it on Windows on 16GB RAM desktop. (But to be honest, to check if this is exactly this file, I will probably check if it reproduces and if so, make a proper bug report :P )

tymond added a comment.EditedJul 8 2019, 10:42 AM

Anna Medonosová wrote:

Note, a non-reproduced hypothesis: Is commit dd14f02b8acb99a316c4f15c5195d0204a0fc7a4 what caused this issue? Previously, all waiting was done after the d->savingMutex was locked. If there is no lock and Krita is closed, the KisDocument destructor will not wait until saving is done.

Could it have something to do with QApplication::processEvents() in KisDocument::lockAndCloneForSaving()? Would it help if it was called with QEventLoop::ExcludeUserInputEvents flag? Are there other places like this?

tymond added a comment.EditedJul 8 2019, 10:43 AM

(About the "Related" links about Dropbox issues and the workaround):

However: the resports we are talking about are usually not related to Dropbox/OneDrive/anything (since we always ask? I hope at least).

On the other hand, it could be that this workaround causes those trouble. The timeframe looks fitting...

Anna Medonosová wrote:

I have my doubts about the workaround. On Linux/Mac we use QSaveFile, which internally tries atomic move of the temporary file (posix rename) before it falls back to copying the file. Also QSaveFile calls fsync (it does not check for errors, but better than nothing :).

Our own implementation, which is turned on for all Windows, does copy the temporary file and I can see no fsync in the process. To me that seems a bit more dangerous than the QSaveFile solution, as there are less saveguards from incomplete writes. Sadly, I know nothing about the Windows API, so I can't say how much better we could get this: for example, Qt uses MoveFile call internally, does that feature atomic move? or, does fsync on windows work the same way it does on linux?

Then there are other questions to thing about:

1, Why are users saving directly to more volatile locations, like USB drives (data loss if the user takes the drive out too early), or network shares (because the network; also drivers for gdrive/dropbox/etc. might have their special idiosyncasies) and should we discourage such usage?

2, Can we detect those more volatile locations and use the workaround only on them, not on windows generally? Does it even make sense?

rempt added a subscriber: rempt.EditedJul 8 2019, 10:49 AM

QSaveFile on Windows triggers the problems with dropbox/onedrive/gdrive where the syncing system locks the temporary file so it cannot be atomically renamed: that's why I implemented the temporary file + copy system. Since it is a copy, the lock problem will not occur. This must have solved the synced folder problem. (The issue is also supposed to be fixed in Qt, but at least our tests showed it still could occur.)

I do not understand how it is possible to not get any warning anymore when saving fails in the ordinary way, because we now explicitly check whether there is a readable file in the destination location.

I also suspect that many of the problems are user errors, like the issue where one user assumed that everything will be fine if they just powered off their system completely, or the user who didn't know what folders were.

Related bug report - a race condition in the saving process leading to missing files: https://bugs.kde.org/show_bug.cgi?id=409395, work in progress fix: https://invent.kde.org/kde/krita/merge_requests/59

I do not understand how it is possible to not get any warning anymore when saving fails in the ordinary way, because we now explicitly check whether there is a readable file in the destination location.

There is at least one level of caching, at the operating system level. Unless we do fsync, the data can be only partially written to the physical device, or not at all. When we check the saved file, we read it from the cache, so everything seems to be ok. With network drives or special filesystems this problem can be even trickier, as they may have different guarantees and implementations.

The question is how far do we want to take it. From my point of view it would be enough to 1, attempt fsync in the code, 2, provide information about the importance of clean system shutdown, safe handling of removable media and network drives to the users.

rempt added a comment.Jul 12 2019, 8:00 AM

Shouldn't a QIODevice::close() do the fsync already?

Shouldn't a QIODevice::close() do the fsync already?

No. There are two operations (in unix terminology): flush and fsync. Flush means commiting in-memory buffers to the OS. Fsync ensures that the OS writes out it's caches to the physical device. QFile::close() calls flushes internally, but it does not do fsync (and it shouldn't do that, at least by default, as that would mean a big performance hit). The only place in Qt where I have found fsyncing is in the QSaveFile, where QFSFileEnginePrivate::nativeSyncToDisk() is called (fsync/fdatasync on unix, FlushFileBuffers on windows).

As a hack for our current workaround, we might be able to call FlushFileBuffers(QFile::handle()). I have not tried whether it's possible to do this way.

I just reported this bug: https://bugs.kde.org/show_bug.cgi?id=410548

Basically, I'd like to see more feedback in the delayed save dialog as right now it's super opaque what is causing the image to be locked. In some cases I actually suspect an update is going awry, but I wouldn't know because there's nothing in the terminal either about what the update scheduler is doing that is locking the image...

after @kamathraghavendra had a 10 minutes of waiting trying to save a 22K by 19K image, @dkazakov said it should be possible to have better saving progress feedback implemented.

There was also a mention that QFileInfo doesn't work on Windows for checking whether a directory is writable... Maybe also related?

That doesn't sound good: https://www.reddit.com/r/krita/comments/cxn349/krita_will_not_open_kra_file/ :( Will gather more information later (right now it's only the post), but I wanted to link it here before I forget.

Writable folder thing should not really be relevant since even if Qt don't know why the saving failed, it should know that it failed and then Krita should know that it failed.

rempt added a comment.Mon, Sep 2, 7:35 AM

Renaming a .kra file as .png won't make any difference, krita will still load it with the kra loader plugin, but it shows the user thinks they're more adept than they are...

tymond added a comment.Thu, Sep 5, 7:42 PM

"Oh no I'm sure I saved but the file is nowhere to be seen" can also be a result of user saving to AppData/Temp because that's where autosaves are being saved, and if you load an autosave, I think Krita later suggest this wild location to save the file as.

rempt added a comment.Thu, Sep 5, 7:51 PM

"Oh no I'm sure I saved but the file is nowhere to be seen" can also be a result of user saving to AppData/Temp because that's where autosaves are being saved, and if you load an autosave, I think Krita later suggest this wild location to save the file as.

That should only be true for autosaves of images that have never been saved before; images that have been saved, have their autosaves saved in the same folder as the image.