Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Think about setting the auto backup feature to off for new installations #6150

Open
dinkumoil opened this issue Sep 15, 2019 · 9 comments
Open

Comments

@dinkumoil
Copy link
Contributor

Description of the Issue

In the community forum a lot of users are complaining about data loss due to the session feature in combination with the auto backup feature. The problem is, that noone knows how to reproduce that failure, thus the bug can not be fixed.

I would like to suggest to turn off the auto backup feature for new installations until this bug has been fixed. An unrelieable feature should not be activated by default.

Debug Information

Applies to all current versions of Notepad++

@Ekopalypse
Copy link
Contributor

Ekopalypse commented Sep 15, 2019

Not really convinced that it is npp related.
Community posts seem to indicate that after a powerloss files have been corrupted,
which is absolutely possible but I still think the action afterwards corrupt the real file.
What I mean might be better explained with an example
Let's assume one is working on a file abc.txt with settings configured for automatic 7 seconds backup.
File gets changed - backup functionality kicks in and creates backup file.
Further changes ... power loss happens during backup function.
File cannot be closed properly and gets corrupted.
User restarts PC and npp and sees corrupted content.
This corrupted content belongs to the backup file but not the original file,
now, user is frustrated and closes this corrupted file which then results in
corruption of the original file.

From my point of view npp could help in one of two ways preventing the user from doing the last action.

  1. There is a flag set when quitting npp to indicate a clean shutdown
    or
  2. Check the content of a backup file before overwriting the original file
  1. is easy to implement I guess.
    On startup read if the flag exists, if yes, delete/overwrite it nothing more to do.
    If it doesn't exists inform the user that an abnormal shutdown has been noticed and the
    user should double check if the files have been corrupted

  2. is much more difficult to implement as there must be some logic introduced to decide that the
    content is useful or not. In case when a file is filled by NULs then it might be easy but what if a file
    is partially corrupted?

My two cents

@dinkumoil
Copy link
Contributor Author

I didn't say that Notepad++ is "the bad guy" which causes data loss, who or what's the cause is totally unclear at the moment. But in fact that feature often leads to data loss for whatever reason, thus IMO it should not be activated by default until the bug is fixed.

@Ekopalypse
Copy link
Contributor

But disabling that feature and until the bug is fixed does indicate that npp is the culprit.
Don't get me wrong, I don't want to be a smartass here.
What do you think about my theory?

@dinkumoil
Copy link
Contributor Author

dinkumoil commented Sep 15, 2019

Hmm, since english is not my mothertongue maybe I wasn't able to express exactly what I meant.

When I talk of "fixing the bug" it is clear that the code of Notepad++ can only be improved in that sense that the changes to be done prevent a data loss. Since I think your theory about what's the cause of the data loss seems to be plausible, that means that a workaround has to be designed to prevent data loss. But until this workaround isn't implemented the auto backup feature should be disabled by default.

Your suggestion 1. (the flag solution) sounds good. But it should be a per-file solution. Maybe for every auto backup file a second file should be created (working as "the flag") after the actual backup file has been written correctly. Thus, before writing to an auto backup file the flag file has to be deleted. Maybe the flag file even could contain a checksum of the backup file.

This solution would allow to consider a backup file as valid if Notepad++ crashed some seconds after the last write operation to that file.

And it means that after installing the update that introduces this feature there has to be run code that creates the flag files for all already existing backup files to flag them as valid.

@Ekopalypse
Copy link
Contributor

Hmm, since english is not my mothertongue maybe I wasn't able to express exactly what I meant.
Ich würde auch lieber in Deutsch schreiben :-D

Yes, as npp does already track session information per file it could be added to session.xml file
and because npp already uses hashing functions ... @donho, what do you think :-) ??

@dinkumoil
Copy link
Contributor Author

dinkumoil commented Sep 15, 2019

Addition: Maybe it's worth to have a look at the ReplaceFile API. This could avoid the need of a flag file.

See also https://docs.microsoft.com/en-us/windows/win32/fileio/deprecation-of-txf#applications-updating-a-single-file-with-document-like-data

@xylographe
Copy link
Contributor

I think, ReplaceFile API will break hard links.

@dinkumoil
Copy link
Contributor Author

@xylographe

Yes, and because of

The backup file, replaced file, and replacement file must all reside on the same volume.

it is not suitable anyway.

@Ekopalypse
Copy link
Contributor

I think a combination of my first suggestion and @dinkumoils suggestion to check per file would be the easiest and maybe most sensible solution.
From a workflow point of view I would say

  • before backup is updated/created get a hash of the current buffer (maybe first 1024 chars is sufficient)
  • create the backup and keep the created hash in the session information details for this file (the one which is used to create the session file)
  • on npp startup check if the hash for the file to be loaded is the same as in session.xml for that file
  • if not, alarm the user that the backup seems to be altered by an external resource and whether the
    original file should be loaded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants