notepad-plus-plus / notepad-plus-plus

Notepad++ official repository
https://notepad-plus-plus.org/
Other
22.91k stars 4.6k forks source link

Reload file message if saved on network drive back in notepad++ v8.5.6 64bit #14103

Open virtualos opened 1 year ago

virtualos commented 1 year ago

Description of the Issue

When I try to save a file over network I always got this reload file window, I even tried to disable it from settings>misc but it didn't help.

Steps to Reproduce the Issue

Try to edit and save a file over network.

Annotation 2023-09-06 020818

donho commented 1 year ago

@virtualos What kind of network ? Could you provide Debug Info via menu ? > Debug Info ?

virtualos commented 1 year ago

Hi,

here is debug info:


Notepad++ v8.5.6 (64-bit) Build time : Aug 15 2023 - 15:29:28 Path : C:\Program Files\Notepad++\notepad++.exe Command Line : "Z:\public_html\tools\New folder\New Text Document.txt" Admin mode : OFF Local Conf mode : OFF Cloud Config : OFF OS Name : Windows 10 Pro (64-bit) OS Version : 22H2 OS Build : 19045.3393 Current ANSI codepage : 1252 Plugins : mimeTools (2.9) NppConverter (4.5) NppExport (0.4)


And Network is my cPanel hosting acount which I use its "Web Disk" feature so I can connect to my hosting space from my windows by adding map network drive:

Map Drive Address:

\\example.com@SSL@2078\DavWWWRoot

xomx commented 1 year ago

@virtualos Could you please create an empty file with the name nppLogNetworkDriveIssue.xml in the Notepad++ installation folder (for you it is the C:\Program Files\Notepad++\) before you start your Notepad++ again?

This is a logging trigger for some potentially problematic network operations. After your issue manifests again, you should obtain the nppLogNetworkDriveIssue.log file in your %APPDATA%\Notepad++\ folder, hopefully with some hints for us. Then please paste this output log-file (or directly its content) here.

xomx commented 1 year ago

@donho

@virtualos is using a WebDAV server folder mapped as a Windows disk letter. I have one such setup available and I can reproduce the problem there, if I try to save a new file there: npp-webdav-filesaving-new-problem

Edit: I retested with the above mentioned nppLogNetworkDriveIssue.xml trigger and I have got this log: 2023-09-07 18:25:08 Z:\NPP\new 2.txt in checkFileState(): attributes.ftLastWriteTime (3720071680/31056295) < _timeStamp (3726005833/31056295)

Edit2: I converted my log-data above to more readable form (FILETIME to SYSTEMTIME), the timestamp difference is 593ms: attributes.ftLastWriteTime 2023/9/7 18:25:8.000 < _timeStamp 2023/9/7 18:25:8.593 (rrrr/mm/dd hh:mm:ss.ms) It means, that the actual file timestamp is a little bit older than the N++ timestamp stored, which is weird.

But when I open an existing file from there, edit it and save, I do not obtain such Reload msgbox: npp-webdav-filesaving-ok

donho commented 1 year ago

@virtualos is using a WebDAV server folder mapped as a Windows disk letter. I have one such setup available and I can reproduce the problem there, if I try to save a new file there But when I open an existing file from there, edit it and save, I do not obtain such Reload msgbox

Thank you @xomx for the information.

@virtualos Do you confirm that you've got exactly the same situation as the one of @xomx ?

donho commented 1 year ago

@virtualos & @xomx Could you try the 3 binaries of #14085 , and report the result in the same thread (#14085) ?

virtualos commented 1 year ago

@virtualos Could you please create an empty file with the name nppLogNetworkDriveIssue.xml in the Notepad++ installation folder (for you it is the C:\Program Files\Notepad++) before you start your Notepad++ again?

This is a logging trigger for some potentially problematic network operations. After your issue manifests again, you should obtain the nppLogNetworkDriveIssue.log file in your %APPDATA%\Notepad++\ folder, hopefully with some hints for us. Then please paste this output log-file (or directly its content) here.

The nppLogNetworkDriveIssue.log does not generate!

@virtualos is using a WebDAV server folder mapped as a Windows disk letter. I have one such setup available and I can reproduce the problem there, if I try to save a new file there But when I open an existing file from there, edit it and save, I do not obtain such Reload msgbox

Thank you @xomx for the information.

@virtualos Do you confirm that you've got exactly the same situation as the one of @xomx ?

YES, It's the same!

donho commented 1 year ago

@virtualos

The nppLogNetworkDriveIssue.log does not generate!

Hmm... following the instructions of @xomx doesn't make generation of nppLogNetworkDriveIssue.log to me either, after saving a file... I will check to see what's wrong in code.

Klotzi111 commented 1 year ago

Hmm... following the instructions of @xomx doesn't make generation of nppLogNetworkDriveIssue.log to me either, after saving a file...

Did you create the nppLogNetworkDriveIssue.log as an empty file manually? Because this file is only written to if it already exists.

Klotzi111 commented 1 year ago

There is an easy fix for the message box to show up (in these "wrong" cases): Generate a checksum over the file content when reading into internal buffer. Store that checksum. Then after receiving the file update event generate the checksum over the "updated" file content and compare the checksums. Then only show the message box if the checksums are not equal.

donho commented 1 year ago

Hmm... following the instructions of @xomx doesn't make generation of nppLogNetworkDriveIssue.log to me either, after saving a file... I will check to see what's wrong in code.

LONG res = CompareFileTime(&_timeStamp, &timeStampLive);
...
    if (res == 1)
    {
        // write the log
    }

Apparently res is never -1 for me, since I work locally. @xomx Is it possible that res is always 1 for the case of @virtualos ? If it is, there must be a trick on the side of WebDAV server, while Notepad++ saving a new file.

donho commented 1 year ago

@Klotzi111

Because this file is only written to if it already exists.

It's not true. The function writeLog() has been tested, and it works always.

Generate a checksum over the file content when reading into internal buffer. Store that checksum.

It's a good idea. However, in the case of a huge file, checksum of the whole content will be very time consuming.

Klotzi111 commented 1 year ago

However, in the case of a huge file, checksum of the whole content will be very time consuming.

I disagree.

No cryptographic checksum (like SHA3) is required. Just use SHA1 or even MD5. This is not security relevant. And computing a SHA1 is so fast. Its faster than reading from an SSD. Also the checksum can be calculated while reading the file in chunks (this requires the read to operate in chunks ofcourse).

But have you opened a big file in N++ before? There are much worse problem than calculating the checksum. The entire file content is loaded 3 times into memory. So opening a 5GB file will require 15GB of heap memory.

If you meant reading the current file content for huge files might take a long time (not the checksum calculation itself) then: Yes probably true. But you probably need to load the file content anyways because the user will likely click "reaload". (For this the old and the new content would need to be in memory. But there would be no difference because as pointed out above this is already the case)

So the "best" solution for this would be: Only do the checksum calculation if the file is smaller than a certain threshold (ie. 100MB)

Klotzi111 commented 1 year ago

Because this file is only written to if it already exists.

It's not true.

Yeah sorry i mistyped. I meant: nppLogNetworkDriveIssue.xml

See the code here: https://github.com/notepad-plus-plus/notepad-plus-plus/blob/5008b8a0cccfff255c5f48b5782ef993b6f9b631/PowerEditor/src/Parameters.cpp#L1583-L1593

and here: https://github.com/notepad-plus-plus/notepad-plus-plus/blob/5008b8a0cccfff255c5f48b5782ef993b6f9b631/PowerEditor/src/ScintillaComponent/Buffer.cpp#L156-L157

xomx commented 1 year ago

@donho

@xomx Is it possible that res is always 1 for the case of @virtualos ?

It seems so. Yesterday I was able to repeat the problem consistently and I always received such a log generated in Buffer::checkFileState(). I will try to do some debugging on that next week (I cannot access my WebDAV server from home anyway).

If it is, there must be a trick on the side of WebDAV server, while Notepad++ saving a new file.

I updated the log report above with a more human readable form. It reminds me of this. In that case, the problem really was at the server side: https://github.com/notepad-plus-plus/notepad-plus-plus/issues/10992#issuecomment-1032380105

virtualos commented 1 year ago

Hello guys,

Thank you all for looking for solutions, I also searched every portion of system(my host's cache system, windows) even I tried to test this issue over VM to be sure of my windows settings and finally found what made this problem!!

Real-time Protection of Windows Security (Microsoft Defender)

I suspect that maybe Windows AV tries to scan open document and when I disable it, Reload window never showed up again; This topic which I read before also helped me to recall: https://www.neowin.net/news/microsoft-defender-update-for-windows-11-or-10-images-improves-performance-blocks-autokms/

Right now I put notepad++.exe process in exclusion list of windows security and problem solved. Anyway, I don't know if there should be any changes in Notepad++ source code for this strange issue but that is what I found.

Regards.

Screenshot 2023-09-09 123015 Screenshot 2023-09-09 123049

donho commented 1 year ago

Thank you @virtualos for finding the cause of this issue and share it with us. It'll be part of our knowledge base for the similar problem.

donho commented 1 year ago

@Klotzi111

If you meant reading the current file content for huge files might take a long time (not the checksum calculation itself) then

Yes. It's what I meant.

But you probably need to load the file content anyways because the user will likely click "reaload". (For this the old and the new content would need to be in memory. But there would be no difference because as pointed out above this is already the case)

Each time for checking the diff, we HAVE TO read the file from the disk for comparing with the current checksum. It'll bring the performance issue. So checksum is not the solution to me.

xomx commented 1 year ago

Thanks @virtualos ! Tomorrow I will try to reproduce your finding.

It is the second time I can see that the MS AV caused a problem for N++ with saving to a network storage. I wonder what is the real reason there (a MS bug or some bad AV/FW setting?).

Klotzi111 commented 1 year ago

It'll bring the performance issue. So checksum is not the solution to me.

And if you only do this for small files? Like <= 500KB (Do nothing special for larger files) This small amount is in the windows/network server cache anyways. So there is almost no performance penalty.

xomx commented 1 year ago

Unfortunately I cannot confirm the @virtualos finding, for me the problem with saving new files is still there:

#14103-MSAVOFF-err

I do not have more time for this right now, but I will try to get back to it one day.

virtualos commented 1 year ago

Unfortunately I cannot confirm the @virtualos finding, for me the problem with saving new files is still there:

#14103-MSAVOFF-err

I do not have more time for this right now, but I will try to get back to it one day.

I'm sorry to hear that. I reopened this issue.

What is your network drive? Is it in your local network or over internet(like mine)? Maybe [just a guess ]you have network AV which scan all files over network!

donho commented 4 months ago

@xomx Can you still reproduce the issue in v8.6.8? Or can we close the issue?

xomx commented 4 months ago

@donho I'll try again when I have that testing NAS available again (in ~2 weeks, I'm on vacation).

donho commented 4 months ago

@xomx Enjoy your holidays!

darkviruzz commented 3 months ago

Hello all,

I'm not sure if my issue is related, but i attach it to this issue, because it is still open and very similar. Also, i did not find any issues related to nextcloud or other "clouds" (if i should create a new issue for that, just tell me :) )

When saving a file in my nextcloud folder, i get the exact same behavior (very reproducible)

what i do to reproduce the issue:

  1. create empty file in actively synced nextcloud folder
  2. open file in npp
  3. make changes
  4. save file
  5. loose focus or change tab
  6. wait 5–10s
  7. get focus or change to tab of the saved file again.
  8. popup notifies me about a change in the file (that actually did not happen, false alarm)

expected: no popup appears after step 7.

Since i regularly change computers from lab to office or private PC and change some files from all locations, I installed autosave plugin. This is when i noticed this issue. but i tested it even without the plugin, it's not the cause. its nevertheless very annoying and because the false alarms are so regular, i even lost some small notes due to just disregarding the popup when it was not false ;-) (nothing serious, but annoying.) And because i regularly change pcs i really like this feature to stay activated.

Can I do anything to help in resolving this issue?


debug info:

Notepad++ v8.6.8   (64-bit)
Build time : Jun  4 2024 - 00:30:00
Path : C:\Users\***\Documents\Software\Notepad++\notepad++.exe
Command Line : "C:\Users\***\Documents\Software\Notepad++\nppLogNetworkDriveIssue.log"
Admin mode : OFF
Local Conf mode : ON
Cloud Config : OFF
Periodic Backup : ON
OS Name : Windows 10 Education (64-bit)
OS Version : 22H2
OS Build : 19045.4170
Current ANSI codepage : 1252
Plugins : 
    AutoCodepage (1.2.7)
    AutoEolFormat (1.0.4)
    AutoSave (2)
    BetterMultiSelection (1.5)
    BigFiles (0.1.3)
    ComparePlus (1.2)
    MenuIcons (2.0.7)
    mimeTools (3.1)
    nppAutoDetectIndent (2.3)
    NppConverter (4.6)
    NppExport (0.4)
    NppMarkdownPanel (0.7.3)
    NppXmlTreeviewPlugin (2)

nppLogNetworkDriveIssue.log

2024-07-29 12:07:44  C:\Users\***\Nextcloud\autosave-test-file.txt  in checkFileState(): attributes.ftLastWriteTime (633099392/31121823) < _timeStamp (641551785/31121823)
2024-07-29 12:08:25  C:\Users\***\Nextcloud\autosave-test-file.txt  in checkFileState(): attributes.ftLastWriteTime (863099392/31121823) < _timeStamp (865029775/31121823)
2024-07-29 12:09:41  C:\Users\***\Nextcloud\autosave-test-file.txt  in checkFileState(): attributes.ftLastWriteTime (1563099392/31121823) < _timeStamp (1565382444/31121823)
2024-07-29 13:16:04  C:\Users\***\Nextcloud\autosave-test-file.txt  in checkFileState(): attributes.ftLastWriteTime (2768393728/31121832) < _timeStamp (2771026985/31121832)
xomx commented 2 months ago

@darkviruzz The following are your 4 log lines above translated to human readable local SYSTEMTIME:

attributes.ftLastWriteTime (2024/07/29 12:07:41.000) < _timeStamp (2024/07/29 12:07:41.845)
attributes.ftLastWriteTime (2024/07/29 12:08:04.000) < _timeStamp (2024/07/29 12:08:04.193)
attributes.ftLastWriteTime (2024/07/29 12:09:14.000) < _timeStamp (2024/07/29 12:09:14.228)
attributes.ftLastWriteTime (2024/07/29 13:15:40.000) < _timeStamp (2024/07/29 13:15:40.263)

In your case the problem is evident - your network layer is not able to return a proper FILETIME (with the miliseconds resolution) for this N++ GetFileAttributesEx WINAPI call: https://github.com/notepad-plus-plus/notepad-plus-plus/blob/72751182bf2a4851f170cb50e503b92eff6fbf2e/PowerEditor/src/ScintillaComponent/Buffer.cpp#L309

I have seen such problems here before. The overall result is - the N++ timestamp method is smart and efficient, but not reliable in such heterogeneous network storage environments. For such an error to show, it is sufficient that e.g. the driver of the network card of such network storage or its OS has a not fully compliant implementation of the response to such a WINAPI call...

I have already posted some workarounds but these were not accepted: https://github.com/notepad-plus-plus/notepad-plus-plus/pull/10847#issuecomment-981152561 . I do not know another (simpler) solution to this yet.

xomx commented 2 months ago

@donho I have another proposal how to fix all of these network storage issues - have a new optional setting in N++ for limiting the N++ timestamp resolution to seconds only (instead of the current milliseconds). From what I have seen so far, it should solve all of these kinds of problems with network storage. What do you think?

donho commented 2 months ago

@xomx Hmm... why not? Where do we put this option IYO?

xomx commented 2 months ago

@donho

npp-MISC-timestamps-mockup

plus a tooltip like: "Use this workaround only if you experience false positive files change detections (common problem for some network storage)."

donho commented 2 months ago

@xomx Firstly, could you please do a PR to remove the millisecond precision completely, so users who have got the network drive problem can test it and confirm their problem is solved?

xomx commented 2 months ago

@donho 

I'd like to but no time (I've to finish my two delayed projects first, sorry). If you are not in a hurry, please assign this to me and I will do it when I can. Otherwise, please do it yourself. I'd proceed in such a way that I'd not touch the method of storing the timestamps at all (session.xml originalFileLastModifTimestamp & originalFileLastModifTimestampHigh) but rather adjust the comparison method on-the-fly (according to that new possible future N++ setting). This way, the precision of the comparison could be toggled back and forth as needed and there also should not occur any regression with the previously stored session.xml data ... 

  

 

---------- Původní e-mail ---------- Od: Don HO @.> Komu: notepad-plus-plus/notepad-plus-plus @. com> Kopie: xomx @.>, Mention @.> Datum: 11. 8. 2024 15:35:43 Předmět: Re: [notepad-plus-plus/notepad-plus-plus] Reload file message if saved on network drive back in notepad++ v8.5.6 64bit (Issue #14103)Message ID: @.***> " "

donho commented 2 months ago

@xomx

I'd like to but no time (I've to finish my two delayed projects first, sorry). If you are not in a hurry, please assign this to me and I will do it when I can. Otherwise, please do it yoursel

OK. No problem.

...but rather adjust the comparison method on-the-fly (according to that new possible future N++ setting).

Thank you for the hint - it's a smart way. I will do a PR with the method you've described.

xomx commented 2 months ago

@donho

Can you still reproduce the issue in v8.6.8? Or can we close the issue?

Today I finally got around to repeating the original issue. I used fresh x64 v8.6.9 N++ for the testing and my previously problematic NAS (that one has been upgraded several times from the last attempt too).

Result - I got that Reload messagebox again but only once at the 1st attempt, then I have not been able to reproduce this error anymore! Weird.

donho commented 2 months ago

@xomx Thank you for the testing.

@darkviruzz Does the issue you encountered still persist? (ie. you can reproduce it all the time)

wiesios commented 3 weeks ago

I have the same problem on version 8.7. I don't want to disable this option because sometimes I actually change something on another workstation and then I need to reload, but if I don't change the computer the message shouldn't appear.

I wonder what the program takes into account to check if the file has changed. Or just the file modification time? Sometimes the server time differs by a few seconds from the station. Maybe there should be an option so that the program does not show the message if the difference is a number of seconds specified in the configuration, or to automatically check if the content really differs before showing the message (i.e. configuration option for files less than [number] MB).

Klotzi111 commented 3 weeks ago

@wiesios I already proposed the idea of checking whether the file actually changed in this thread (see above). But the maintainers (for some reason) didn't really like the idea and then it was ignored.

But maybe we can change that if enough people think is the way to go.

xomx commented 3 weeks ago

@wiesios

I wonder what the program takes into account to check if the file has changed. Or just the file modification time?

Yes, it uses checking of the file timestamp only.

Sometimes the server time differs by a few seconds from the station.

It does not matter, N++ just compares two such server filetimes reported (it will not compare the server filetime against your machine time...)

or to automatically check if the content really differs before showing the message

Yes, that is another possible sln. But I still think that the option to limit the file timestamp resolution to seconds (instead of milliseconds) will suffice here.