nextcloud / desktop

💻 Desktop sync client for Nextcloud
https://nextcloud.com/install/#install-clients
GNU General Public License v2.0
2.97k stars 784 forks source link

Excel says the document might have been saved by another person after some time #3456

Closed claell closed 9 months ago

claell commented 3 years ago

How to use GitHub

Expected behaviour

Excel documents should save without problems

Actual behaviour

When having an Excel document open for some time and then trying to save it, a message appears saying that the file might have been modified by another person (which did not happen)

Steps to reproduce

  1. Use the Virtual Filesystem on Windows
  2. Open an Excel file (save it)
  3. Wait some time
  4. Try to save it again

Client configuration

Client version: 3.2.2

Operating system: Windows 10

OS language: German

Qt version used by client package (Linux only, see also Settings dialog):

Client package (From Nextcloud or distro) (Linux only):

Installation path of client:

Server configuration

Nextcloud version:

Storage backend (external storage):

Logs

Please use Gist (https://gist.github.com/) or a similar code paster for longer logs.

  1. Client logfile: Since 3.1: Under the "General" settings, you can click on "Create Debug Archive ..." to pick the location of where the desktop client will export the logs and the database to a zip file. On previous releases: Via the command line: nextcloud --logdebug --logwindow or nextcloud --logdebug --logfile log.txt (See also https://docs.nextcloud.com/desktop/3.0/troubleshooting.html#log-files)

  2. Web server error log:

  3. Server logfile: nextcloud log (data/nextcloud.log):

claell commented 3 years ago

Here is a German entry describing a similar problem with Excel and OneDrive: https://answers.microsoft.com/de-de/msoffice/forum/msoffice_excel-mso_win10-mso_365hp/excel-in-onedrive-durch-einen-anderen-benutzer/2fec7b7b-0114-4de5-99f3-952734afba4d?auth=1.

claell commented 3 years ago

This issue seems only to happen extremely rarely for me now. Maybe once or twice time since the last time I posted here.

github-actions[bot] commented 3 years ago

This bug report did not receive an update in the last 4 weeks. Please take a look again and update the issue with new details, otherwise the issue will be automatically closed in 2 weeks. Thank you!

claell commented 3 years ago

Still relevant. Just no dev has replied so far. It is not ideal that bugs are marked as stale in those cases.

dahooz commented 3 years ago

I have an observation that might help finding the reason for this issue.

For me, this problem occurs with version 3.3.2 on Windows for .tex files when using TeXStudio. The process is as follows:

  1. I save a tex file from TeXStudio in some subfolder of the nextcloud sync folder, which is set to "virtual-mode".
  2. after some seconds, Nextcloud Client notes the change and initiates sync (it must upload local to the server)
  3. Immediately when I see the sync happening in nextcloud client settings window: TeXStudio complains, that there was an external change to the file.

I observed this situation in parallel with the Windows explorer: The reason might be a change on the creation date of the file. When TeXStudio saves the file, only the modification date and access date gets updated. As soon as Nextcloud Client syncs, the creation date is updated, too (to the same time as the modification date and access date). This seems to trigger the warning about an external change.

I have no idea of the WinAPI, but maybe you can not only set the mod time (example code from downloading a file, where you update mod time), but also backup and set the creation time?

Another reason could be a logic error: Could it be, that a file change (maybe a move or sth.) is triggered even if local is the most recent change? I can not see anything in propagateupload.cpp (also not in *ng.cpp or v1.cpp) that could explain this. But I could imagine that despite local being the "newest version", still somehow something gets active that changes creation date, eg. Nextcloud Client downloads the file again, or triggers some slot related to downlaod?

claell commented 2 years ago

@dahooz Thanks for your observations! I am not sure, whether it explains the Excel problem, though (as the problem does not happen every time, in fact rather rarely, currently).

Still, the behavior of changing the creation date seems to be pretty wrong. Possibly this might even be worth a separate bug report to raise awareness.

Ping @allexzander @FlexW

FlexW commented 2 years ago

@dahooz Thanks for the valuable information. Maybe you can open an extra ticket for it when you have clear steps to reproduce the problem.

lars-sh commented 2 years ago

To be honest, the same thing @dahooz described for TeXStudio happens with simple text files when saving them using Notepad++ to me - on a regular and reproducable basis.

In addition I can mention, that I'm using Nextcloud 22.2.0 with the "Client Push" app in version 0.2.4 and version 3.3.5 of the Nextcloud desktop client on Windows 11, even though the same happened on Windows 10, too.

claell commented 2 years ago

@FlexW I know you are not working for Nextcloud anymore, but I think there are already clear steps for reproduction in the comment from @dahooz.

Apparently, this problem also happens for other files, according to @lars-sh. So maybe somebody can take a closer look, as this might explain some strange behavior of the sync and should also not happen.

donaldsycamore commented 2 years ago

Is this related to what I'm seeing with mtimes being truncated: can be reproduced at the bash/msys commandline (Windows 11, Nextcloud Desktop 3.4.1):

QOwnNotes is affected by this too. I see that Nextcloud is truncating file timestamps -- running this in msys on Windows 11 I see after a few seconds the mtime changes:

donald@GIGABYTE-AERO MSYS /c/users/donald/nextcloud/test
$ while [ 1 ] ; do echo "touch `date`" ; touch testfile2; for i in `seq 1 10` ; do ls -l --time-style=full-iso testfile2;sleep 1;done; done
touch Wed, Jan 26, 2022 10:51:31 PM
-rw-r--r-- 1 donald donald 0 2022-01-26 22:51:31.872618400 +1300 testfile2
-rw-r--r-- 1 donald donald 0 2022-01-26 22:51:31.872618400 +1300 testfile2
-rw-r--r-- 1 donald donald 0 2022-01-26 22:51:31.872618400 +1300 testfile2
-rw-r--r-- 1 donald donald 0 2022-01-26 22:51:31.872618400 +1300 testfile2
-rw-r--r-- 1 donald donald 0 2022-01-26 22:51:31.000000000 +1300 testfile2
-rw-r--r-- 1 donald donald 0 2022-01-26 22:51:31.000000000 +1300 testfile2
-rw-r--r-- 1 donald donald 0 2022-01-26 22:51:31.000000000 +1300 testfile2
-rw-r--r-- 1 donald donald 0 2022-01-26 22:51:31.000000000 +1300 testfile2
-rw-r--r-- 1 donald donald 0 2022-01-26 22:51:31.000000000 +1300 testfile2
claell commented 2 years ago

@donaldsycamore Thanks for the hint! This might indeed cause it, but I am not sure. Let's wait what the devs say.

@allexzander @mgallien can you reproduce this? Is this behavior wanted?

donaldsycamore commented 2 years ago

It looks like

So it's not just virtual files that is truncating the mtime, it's just that without virtual files that truncation is not visible on the machine where the change was made.

The file on the server has its mtime truncated too, so this is happening as the file is written to the server. Is the sync protocol capable of sending the fraction-of-a-second mtime?

claell commented 2 years ago

Also note the comment above: https://github.com/nextcloud/desktop/issues/3456#issuecomment-914701354

There it is about creation times. Have you checked these as well?

dahooz commented 2 years ago

I think @donaldsycamore 's analysis leads somewhere:

https://github.com/nextcloud/desktop/blob/fe799e983bf22f64d20ae4625ae0d77f7b5aa06e/src/libsync/filesystem.cpp#L73-L85

here the u_seconds for both modification time and creation time times[0] and [1] get set to zero.

*This spoiler covers old, probably wrong information* This function gets called (only?) for the Virtual file system synchronization here: https://github.com/nextcloud/desktop/blob/5e651878ff02514aa802e936d49b1b5626996ff7/src/libsync/vfs/xattr/vfs_xattr.cpp#L70-L78 and here https://github.com/nextcloud/desktop/blob/5e651878ff02514aa802e936d49b1b5626996ff7/src/libsync/vfs/suffix/vfs_suffix.cpp#L69-L77 Which would explain why this is a problem only for the virtual file system.

the c_utimes is defined here, and I do not see a reason why u-seconds truncation is necessary.

Hunting down some git blames on the line that truncates u-seconds explains nothing, unfortunately. The usecond truncation was inserted some time ago with this commit

Edit: @donaldsycamore:

If you modify a file in a virtual files sync folder, for ~5s the mtime is not trucated and then it is

As far as I have understood, this is due to the sync client always "resynchronizing" files after uploading them, at least that's what I remember that I concluded when trying to debug this in september. Unfortunately I just didn't see the u-seconds truncation code when I last investigated this.

Edit 2: I think this the above locations of setModTime (in the spoiler tag) might not be the cause, and it rather makes sense to look over here: https://github.com/nextcloud/desktop/blob/2e0ba8208b4597d5c6b0d338a80a8b0eec2910ae/src/libsync/propagatedownload.cpp#L605

donaldsycamore commented 2 years ago

I get the feeling that Nextcloud/its predecessors simply don't expect to preserve subsecond mtimes. Most of the files in my data/(user)/files folder on my server have truncated mtimes. The oc_filecache table represents the mtime as a bigint. Filesystem::setModTime() there is getting the modification time as a time_t, i.e. integer.

The options for fixing this then are either to make all of Nextcloud care about subsecond mtimes, or for the client to have a workaround where when resynchronizing an unmodified file it retains the tv_usec part of the mtime.

Such a workaround could be https://github.com/donaldsycamore/desktop/commit/a295b0c239c7f2627edc4d81647b2774fc62aedb

I'm having trouble getting builds working locally so if someone could try out the patch and confirm it works (use testing method in https://github.com/nextcloud/desktop/issues/3456#issuecomment-1022035018) that would be wonderful.

mgallien commented 2 years ago

I get the feeling that Nextcloud/its predecessors simply don't expect to preserve subsecond mtimes. Most of the files in my data/(user)/files folder on my server have truncated mtimes. The oc_filecache table represents the mtime as a bigint. Filesystem::setModTime() there is getting the modification time as a time_t, i.e. integer.

The options for fixing this then are either to make all of Nextcloud care about subsecond mtimes, or for the client to have a workaround where when resynchronizing an unmodified file it retains the tv_usec part of the mtime.

Such a workaround could be donaldsycamore@a295b0c

I'm having trouble getting builds working locally so if someone could try out the patch and confirm it works (use testing method in #3456 (comment)) that would be wonderful.

Thanks for your work on this !

you can open a pull request as part of the automated checks pipeline, we trigger a build for Linux and for Windows you would then get automated tests executed and we should at least see if nothing covered by them gets broken.

claell commented 2 years ago

An important thing to clarify is whether this is wanted or not. Of course, a workaround is nice, but it would be also nice to know whether the workaround is the actual solution or whether the actual solution is to make Nextcloud care about subsecond mtimes.

Pinging @tobiasKaminsky and also some people from the server side @nickvergessen @CarlSchwan @szaimen @skjnldsv

WhiteChairFromIkea commented 2 years ago

This issue should be renamed to "File is changed / touched after save in VFS (virtual file system)" or something like that, because this bug applies to any file, no excel relevant (or should I open another one?). Here are my steps to reproduce:

  1. Have an txt file anywhere in VFS folder;
  2. Open, edit, save it with text editor, capable of file change monitoring in the background (notepad++, notepad3);
  3. Wait a bit. File just saved gets "touched" by nextcloud in some way (file attributes not changed, modification dates also, seems normal..);
  4. Inside notepad: Get a notification that file is changed (aka "Do you want to reload file from disk?");

See the screencast: my file explorer (DC commander highlights files which are "touched" in some way. See how txt file is touched unnecessary 2nd time after save:

https://user-images.githubusercontent.com/46539653/162402085-273293ad-1449-4f4a-a91c-de35fe9e0a95.mp4

Expected behavior: File properties and content is not modified by nextcloud itself in any way. Follow the same steps to reproduce on Google VFS -> the issue is not present.

Client version:
3932a (I am stuck at this version, because https://github.com/nextcloud/server/issues/3056#issuecomment-1092619556 and https://github.com/nextcloud/server/issues/30263 are not solved still (cannot risk to lose files in any way).)

dsahlberg-apache-org commented 1 year ago

There seems to be a pull request https://github.com/nextcloud/desktop/pull/4236

Is there anything I can do to help push this forward?

bbceg commented 1 year ago

Just to chip in (and apologies if this has already been made clear and I've missed it) but I'm seeing this without virtual folders. Perhaps the proposed fix doesn't care...

Some context.

tomdereub commented 1 year ago

Same issue for me with excel 2021 on windows 10 pro, nextcloud desktop client 3.9.0 (without VFS) and Nextcloud 25.0.6.

image

Steps to reproduce exactly like said in the first post.

roberix commented 1 year ago

Hi.

Same issue with client 3.9.0 with VSF on NC 25.0.8 using Windows 10 and Office 2019.

Regards

rob

bbceg commented 1 year ago

I don't want to jump the gun but I think this may have been fixed in 3.9.2 (though I don't use the Virtual Filesystem so perhaps it has only resolved the issue for local folders).

Anyone else care to verify in case we can close this one?

roberix commented 1 year ago

Hi.

Yes. We have been plagued by this. Gone since 3.9.2 and we use VFS.

Regards.

Rob

mgallien commented 10 months ago

I posted a test build in the linked PR, people could you please test it and report your results here ? that would be greatly appreciated and useful

Mika-6-9 commented 9 months ago

Hi. Same issue with client 3.10.2 with VSF on NC 27.1.1 using Windows 11. It's not closed by #6212 Regards

Mika-6-9 commented 9 months ago

Hi. Same issue with client 3.11.0 with VSF on NC 27.1.1 using Windows 11. It's not closed by https://github.com/nextcloud/desktop/pull/6263

I don't understand why it doesn't work, normally the change is included since version 3.10.2 Regards