Bug 392408

Summary: File doesnt save if google drive is open and actively syncing
Product: [Applications] krita Reporter: Bollebib <kwadraatnope>
Component: File formatsAssignee: Krita Bugs <krita-bugs-null>
Status: RESOLVED DUPLICATE    
Severity: critical CC: alvin, dimula73, halla
Priority: NOR Keywords: regression, release_blocker
Version: 4.0   
Target Milestone: ---   
Platform: Microsoft Windows   
OS: Microsoft Windows   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Bollebib 2018-03-27 12:38:10 UTC
Save and autosave doesnt work or work well when google drive is open and syncing



https://www.youtube.com/watch?v=f773Qn2VGjc&feature=youtu.be



This has only started with 4.0 and is likely due to the new manner in which krita saves.
Possibly only happens with bigger files,not sure. Needs to be tested.
Possible only in googledrive,but maybe dropbox should be tested too.



Requirements to trigger bug
1)krita file open
2) modify
3) make sure google drive is open and active
4) save
->most likely saving wont work till you disable or close google drive
Comment 1 Halla Rempt 2018-03-27 12:39:07 UTC
See 

https://bugreports.qt.io/browse/QTBUG-60848?jql=text%20~%20%22qsavefile%22

https://bugreports.qt.io/browse/QTBUG-57299?jql=text%20~%20%22qsavefile%22

We should text the attached patch and ship that with our Qt.
Comment 2 Halla Rempt 2018-03-27 14:08:15 UTC
Git commit 9d63ca956a74804141ce93ffbe3653a969e2c5ef by Boudewijn Rempt.
Committed on 27/03/2018 at 14:02.
Pushed by rempt into branch 'master'.

Return an error if QSaveFile cannot commit.
Related: bug 392410

M  +3    -1    libs/ui/KisImportExportManager.cpp

https://commits.kde.org/krita/9d63ca956a74804141ce93ffbe3653a969e2c5ef
Comment 3 Halla Rempt 2018-03-28 11:07:36 UTC
Okay, the patch cannot be applied to Qt 5.9. Too much has changed... The only thing we can do is stop using QSaveFile, at least on Windows.
Comment 4 Alvin Wong 2018-03-28 11:15:13 UTC
That patch doesn't seem to touch a lot of code, perhaps I can give it a go?
Comment 5 Halla Rempt 2018-03-28 11:19:15 UTC
Sure, I wouldn't mind that at all :-) I just couldn't figure out what earlier changes were necessary to also port over, and copying the entire files ran into other missing includes.
Comment 6 Halla Rempt 2018-03-28 11:23:03 UTC
On Twitter, someone also gave us their savefile abstractions: 

https://github.com/bjorn/tiled/commit/827663304e6ed69604d00d07b5d49f947119a713
Comment 7 Bollebib 2018-03-28 14:45:43 UTC
just as an added finding and possibly related


if krita is open,and you save that file

and during that time you also open that file in another instance of krita  (needs to be a big file) 
then the same issue will occur. The save will conclude,but no actual save has been done.

This is very edge case,but i think there still should be a warning if the save failed.
Comment 8 Alvin Wong 2018-03-28 15:15:37 UTC
(In reply to Bollebib from comment #7)
> just as an added finding and possibly related
> 
> 
> if krita is open,and you save that file
> 
> and during that time you also open that file in another instance of krita 
> (needs to be a big file) 
> then the same issue will occur. The save will conclude,but no actual save
> has been done.
> 
> This is very edge case,but i think there still should be a warning if the
> save failed.

Shouldn't 9d63ca956a74804141ce93ffbe3653a969e2c5ef fix this case though?
Comment 9 Alvin Wong 2018-03-28 15:23:16 UTC
I suggest making a test script to try to reproduce the issue. The script should monitor a directory to open any new files for reading (which should lock the file) and keep it opened. Perhaps with the help of something like https://pypi.python.org/pypi/watchdog
Comment 10 Alvin Wong 2018-03-29 13:55:10 UTC
This script should be able to reproduce the issue: https://phabricator.kde.org/P182

Run the script before saving and observe with Process Monitor.
Comment 11 Alvin Wong 2018-03-29 14:51:54 UTC
Tested with the above script and Boud's build with patched Qt and I can confirm that the other processes were prevented from locking the temporary file, at least when it was being opened by Krita.

Though I observed that Qt would still need to close the file before being able to perform the rename operation. There leaves a very small time window for other processes to possibly lock the file and prevent the move, so QSaveFile::commit could still report failure. But at least it's way less likely to happen with the patch.

We still need verification that it does work with the Dropbox sync client though. I'd be interested to see a Process Monitor capture showing both krita and the dropbox client accessing the file when saving.
Comment 12 Halla Rempt 2018-04-03 11:46:43 UTC
Git commit 00951bc9f919d5d926fbc87000df92da15054ec5 by Boudewijn Rempt.
Committed on 03/04/2018 at 11:16.
Pushed by rempt into branch 'krita/4.0'.

Return an error if QSaveFile cannot commit.
Related: bug 392410
(cherry picked from commit 9d63ca956a74804141ce93ffbe3653a969e2c5ef)

M  +3    -1    libs/ui/KisImportExportManager.cpp

https://commits.kde.org/krita/00951bc9f919d5d926fbc87000df92da15054ec5
Comment 13 Halla Rempt 2018-04-11 10:09:14 UTC
Fixed by patching Qt.
Comment 14 Alvin Wong 2018-04-13 13:23:21 UTC
The Qt patch apparently wasn't committed to both the nightly and the 4.0.1 builds...
Comment 15 Alvin Wong 2018-04-13 13:50:00 UTC
Git commit e3c358a9b0ff0286f122b3e9a542d82d5448362e by Alvin Wong, on behalf of Boudewijn Rempt.
Committed on 13/04/2018 at 13:44.
Pushed by alvinwong into branch 'master'.

Win: Patch Qt to fix QSaveFile locking with Dropbox

Add patch for https://bugreports.qt.io/browse/QTBUG-57299

M  +1    -0    3rdparty/ext_qt/CMakeLists.txt
A  +112  -0    3rdparty/ext_qt/QTBUG-57299.diff

https://commits.kde.org/krita/e3c358a9b0ff0286f122b3e9a542d82d5448362e
Comment 16 Alvin Wong 2018-04-13 13:51:34 UTC
Git commit 572348686b82424b5b70b2a3a75062e430e1e972 by Alvin Wong, on behalf of Boudewijn Rempt.
Committed on 13/04/2018 at 13:51.
Pushed by alvinwong into branch 'krita/4.0'.

Win: Patch Qt to fix QSaveFile locking with Dropbox

Add patch for https://bugreports.qt.io/browse/QTBUG-57299
(cherry picked from commit e3c358a9b0ff0286f122b3e9a542d82d5448362e)

M  +1    -0    3rdparty/ext_qt/CMakeLists.txt
A  +112  -0    3rdparty/ext_qt/QTBUG-57299.diff

https://commits.kde.org/krita/572348686b82424b5b70b2a3a75062e430e1e972
Comment 17 Alvin Wong 2018-04-14 07:06:03 UTC
Git commit 5f6e9e9eac42163f045c8edf9ff09ebf17e7181d by Alvin Wong.
Committed on 14/04/2018 at 07:05.
Pushed by alvinwong into branch 'master'.

Win: Fix the Qt patch for QSaveFile

Fix the patch added in e3c358a9b0ff0286f122b3e9a542d82d5448362e

M  +41   -42   3rdparty/ext_qt/QTBUG-57299.diff

https://commits.kde.org/krita/5f6e9e9eac42163f045c8edf9ff09ebf17e7181d
Comment 18 Dmitry Kazakov 2018-04-16 10:10:42 UTC
Hi, Bollebib! Could you please check if this build fixes the saving problem for you?

https://yadi.sk/d/CZZgS2NS3URtZZ
Comment 19 Alvin Wong 2018-04-16 12:33:01 UTC
Git commit a5c3f39157f9d837d631128f52ac5944e26b9400 by Alvin Wong.
Committed on 16/04/2018 at 12:32.
Pushed by alvinwong into branch 'krita/4.0'.

Win: Fix the Qt patch for QSaveFile

Fix the patch added in e3c358a9b0ff0286f122b3e9a542d82d5448362e
(cherry picked from commit 5f6e9e9eac42163f045c8edf9ff09ebf17e7181d)

M  +41   -42   3rdparty/ext_qt/QTBUG-57299.diff

https://commits.kde.org/krita/a5c3f39157f9d837d631128f52ac5944e26b9400
Comment 20 Dmitry Kazakov 2018-04-16 14:11:17 UTC
Hi, Bollebib!

Here is a new package with fixed menus. Please check it, and if the bug is still there, reopen the bug:

https://yadi.sk/d/DUmWS5cr3UTZf6
Comment 21 Alvin Wong 2018-07-25 14:43:32 UTC

*** This bug has been marked as a duplicate of bug 394376 ***