Prevent file corruption in some cases · Pull Request #120 · notepad-plus-plus/notepad-plus-plus

ghost · 2015-05-26T14:16:19Z

Create a file with 100 000 GUIDs (any large file will do, really).
Open the file in Notepad++ and enable session snapshot.
Set backup time to 1 second.
Replace every "a" with "b" in the file.
Observe corruption (the CR/LFs):

FileManager::backupCurrentBuffer() guards against being called at the same time from different threads, but it does nothing to prevent non-threadsafe access to Scintilla etc.

In an ideal world, this feature would be turned off until it can be implemented safely (it looks like a lot of work to me, but maybe someone can think of an easy solution). I don't think that's going to happen, so as a temporary fix I've added a mutex which will prevent corruption in some cases (undo, redo, replace, sort).

The session snapshot feature runs in its own thread and access to Scintilla etc is not thread-safe. As a *temporary* and *non-exhaustive* fix we guard some long-running operations (undo, redo, replace, sort) with a mutex to prevent data corruption.

ariccio · 2015-05-26T20:34:35Z

This is a desperately needed fix to a serious issue. You're awesome

ariccio · 2015-05-26T20:40:27Z

Three questions:

what's with the M30_IDE_ in M30_IDE_LONGRUNNINGOPERATION?
why is longRunningOperation a class if everything's public?
I assume that _operationMutex is a bikeshed? Otherwise it should be static or in an anonymous namespace?

ariccio · 2015-05-26T20:46:28Z

Xref: Issue #24

ghost · 2015-05-26T20:59:43Z

Don't actually know, I just imitated some other header.
I generally only use structs like "dumb groupings of variables" like you would in straight C. Here we want RAII logic in the constructor/destructor, so no struct even though it is possible.
Changed it to static, good catch.

ariccio · 2015-05-27T02:44:20Z

Don't actually know, I just imitated some other header.

snickers quietly

(2) is definitely my C background speaking..... but using a struct doesn't prevent RAII behavior?? Saying struct longRunningOperation is not functionally different than class longRunningOperation.

struct longRunningOperation however, has public inheritance by default. Thus if someone needs to derive from it, struct anotherRunningOperation : longRunningOperation is the same as struct anotherRunningOperation : public longRunningOperation.

ariccio · 2015-05-27T02:47:39Z

An alternative, but messy fix: mark FileManager::backupCurrentBuffer() as _No_competing_thread_, use locking to protect calling from multiple threads, and verify with static analysis.

ghost · 2015-05-27T06:48:40Z

but using a struct doesn't prevent RAII behavior

I know. What I mean is that I only want structs as dumb groupings of variables. Anything else gets to be a class. It's akin to Hungarian notation or prefixing member variables with "m_".

ariccio · 2015-05-27T18:36:25Z

What I mean is that I only want structs as dumb groupings of variables.

Fair enough. As long as you're not using it instead of std::is_pod.

See Sourceforge #5327.

TeoTwawki · 2015-05-30T08:23:45Z

This is a desperately needed fix to a serious issue. You're awesome

Seconded! Awesomeness +2
Just had this bug wreck 20k lines of a large file on me, because I didn't realize the lower half of my file had been corrupted after one tiny little change mid file. I'm going to have to get used to manually saving my stuff and turn the session snapshots off.

[BUG_FIXED] Prevent big file corruption on some long period operations.

donho · 2015-05-30T16:00:40Z

Nice patch!

DmFedorov · 2015-06-19T02:47:55Z

New version 6.7.9 with this patch have problems.

The Font is other, and I cannot change it. Therefore I cannot use this version of Np++. Font is very important and it never was changed.
Np++ loading of CPU is so intensive that first 35 seconds I cannot see Settings dialog. And all this only with one not big file.
So must I proof this patch? No! I cannot.

ghost · 2015-06-19T07:12:30Z

@DmFedorov The patch was pulled out of 6.7.9 due to compatibility issues with WinXP, so it can't very well be the cause of those problems! (It will be in the next version.) Number one is due to #116, don't know about the other one. Maybe start a new issue?

DmFedorov · 2015-06-19T13:47:15Z

OK. I checked the new version. Yes, the patch is not used.
Everything works as before, with the only difference that works worse.
Plus I must read a whole bunch of others' opinions to find out how to disable Clear type font (if at all is possible) because I see in "#116" that nobody have said "how".
I have never used font Clear type, and suddenly the font is turned on itself. This never happened in all history of Np ++.

update to https://www.scintilla.org/scintilla533.zip with: Released 8 February 2023. Fix SCI_LINESJOIN bug where carriage returns were incorrectly retained. Bug notepad-plus-plus#2372. Fix SCI_VERTICALCENTRECARET to update the vertical scroll position. When an autocompletion list is shown in response to SCN_CHARADDED, do not process character as fill-up or stop. This avoids closing immediately when a character may both trigger and finish autocompletion. On Cocoa fix character input bug where dotless 'i' and some other extended Latin characters could not be entered. The change also stops SCI_ASSIGNCMDKEY from working with these characters on Cocoa. Bug notepad-plus-plus#2374. On GTK, support IME context. Feature notepad-plus-plus#1476. On GTK on Win32, fix scrolling speed to not be too fast. Bug notepad-plus-plus#2375. On Qt, fix indicator drawing past left of text pane over margin. Bug notepad-plus-plus#2373, Bug notepad-plus-plus#1956. On Qt, allow scrolling with mouse wheel when scroll bar hidden. and https://www.scintilla.org/lexilla522.zip with Released 8 February 2023. C++: Fix keywords that start with non-ASCII. Also affects other lexers. Issue notepad-plus-plus#130. Matlab: Include more prefix and suffix characters in numeric literals. Issue notepad-plus-plus#120. Matlab: More accurate treatment of line ends inside strings. Matlab and Octave are different here. Issue notepad-plus-plus#18. Modula-3: Don't treat identifier suffix that matches keyword as keyword. Issue notepad-plus-plus#129. Modula-3: Fix endless loop in folder. Issue notepad-plus-plus#128. Modula-3: Fix access to lines beyond document end in folder. Issue notepad-plus-plus#131. Python: Don't highlight match and case as keywords in contexts where they probably aren't used as keywords. Pull request notepad-plus-plus#122. X12: Support empty envelopes. Bug notepad-plus-plus#2369.

update to https://www.scintilla.org/scintilla533.zip with: Released 8 February 2023. Fix SCI_LINESJOIN bug where carriage returns were incorrectly retained. Bug notepad-plus-plus#2372. Fix SCI_VERTICALCENTRECARET to update the vertical scroll position. When an autocompletion list is shown in response to SCN_CHARADDED, do not process character as fill-up or stop. This avoids closing immediately when a character may both trigger and finish autocompletion. On Cocoa fix character input bug where dotless 'i' and some other extended Latin characters could not be entered. The change also stops SCI_ASSIGNCMDKEY from working with these characters on Cocoa. Bug notepad-plus-plus#2374. On GTK, support IME context. Feature notepad-plus-plus#1476. On GTK on Win32, fix scrolling speed to not be too fast. Bug notepad-plus-plus#2375. On Qt, fix indicator drawing past left of text pane over margin. Bug notepad-plus-plus#2373, Bug notepad-plus-plus#1956. On Qt, allow scrolling with mouse wheel when scroll bar hidden. and https://www.scintilla.org/lexilla522.zip with Released 8 February 2023. C++: Fix keywords that start with non-ASCII. Also affects other lexers. Issue notepad-plus-plus#130. Matlab: Include more prefix and suffix characters in numeric literals. Issue notepad-plus-plus#120. Matlab: More accurate treatment of line ends inside strings. Matlab and Octave are different here. Issue notepad-plus-plus#18. Modula-3: Don't treat identifier suffix that matches keyword as keyword. Issue notepad-plus-plus#129. Modula-3: Fix endless loop in folder. Issue notepad-plus-plus#128. Modula-3: Fix access to lines beyond document end in folder. Issue notepad-plus-plus#131. Python: Don't highlight match and case as keywords in contexts where they probably aren't used as keywords. Pull request notepad-plus-plus#122. X12: Support empty envelopes. Bug notepad-plus-plus#2369. update CMakeLists.txt to latest changes within vcxproj file

update to https://www.scintilla.org/scintilla533.zip with: 1. Released 8 February 2023. 2. Fix SCI_LINESJOIN bug where carriage returns were incorrectly retained. Bug #2372. 3. Fix SCI_VERTICALCENTRECARET to update the vertical scroll position. 4. When an autocompletion list is shown in response to SCN_CHARADDED, do not process character as fill-up or stop. This avoids closing immediately when a character may both trigger and finish autocompletion. 5. On Cocoa fix character input bug where dotless 'i' and some other extended Latin characters could not be entered. The change also stops SCI_ASSIGNCMDKEY from working with these characters on Cocoa. Bug #2374. 6. On GTK, support IME context. Feature #1476. 7. On GTK on Win32, fix scrolling speed to not be too fast. Bug #2375. 8. On Qt, fix indicator drawing past left of text pane over margin. Bug #2373, Bug #1956. 9. On Qt, allow scrolling with mouse wheel when scroll bar hidden. and https://www.scintilla.org/lexilla522.zip with 1. Released 8 February 2023. 2. C++: Fix keywords that start with non-ASCII. Also affects other lexers. Issue #130. 3. Matlab: Include more prefix and suffix characters in numeric literals. Issue #120. 4. Matlab: More accurate treatment of line ends inside strings. Matlab and Octave are different here. Issue #18. 5. Modula-3: Don't treat identifier suffix that matches keyword as keyword. Issue #129. 6. Modula-3: Fix endless loop in folder. Issue #128. 7. Modula-3: Fix access to lines beyond document end in folder. Issue #131. 8. Python: Don't highlight match and case as keywords in contexts where they probably aren't used as keywords. Pull request #122. 9. X12: Support empty envelopes. Bug #2369. update CMakeLists.txt to latest changes within vcxproj file Close #13082

Add "static" to get internal linkage.

5b28e27

ariccio mentioned this pull request May 28, 2015

Bug in Notepad ++ for large files #24

Closed

Also sync when pasting.

3ca488d

See Sourceforge #5327.

donho added a commit that referenced this pull request May 30, 2015

Merge pull request #120 from andreas-jonsson/tempfix_corruption

17e8ca3

[BUG_FIXED] Prevent big file corruption on some long period operations.

donho merged commit 17e8ca3 into notepad-plus-plus:master May 30, 2015

ghost deleted the tempfix_corruption branch June 1, 2015 17:02

milipili modified the milestone: 6.x Jun 29, 2015

chcg mentioned this pull request Feb 9, 2023

Update to Scintilla 5.3.3 and Lexilla 5.2.2 #13082

Closed

Conversation

ghost commented May 26, 2015

Uh oh!

ariccio commented May 26, 2015

Uh oh!

ariccio commented May 26, 2015

Uh oh!

ariccio commented May 26, 2015

Uh oh!

ghost commented May 26, 2015

Uh oh!

ariccio commented May 27, 2015

Uh oh!

ariccio commented May 27, 2015

Uh oh!

ghost commented May 27, 2015

Uh oh!

ariccio commented May 27, 2015

Uh oh!

TeoTwawki commented May 30, 2015

Uh oh!

donho commented May 30, 2015

Uh oh!

DmFedorov commented Jun 19, 2015

Uh oh!

ghost commented Jun 19, 2015

Uh oh!

DmFedorov commented Jun 19, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants