From 2a7ef2ecf01ac7027289818b6e12d3af8fdde6b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20J=C3=B6nsson?= Date: Tue, 26 May 2015 15:58:46 +0200 Subject: [PATCH 1/3] Guard long-running operations with a mutex 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. --- .../src/MISC/Common/LongRunningOperation.cpp | 42 +++++++++++++++++++ .../src/MISC/Common/LongRunningOperation.h | 39 +++++++++++++++++ PowerEditor/src/NppCommands.cpp | 9 ++++ PowerEditor/src/ScitillaComponent/Buffer.cpp | 3 ++ .../src/ScitillaComponent/FindReplaceDlg.cpp | 5 +++ PowerEditor/visual.net/notepadPlus.vcxproj | 1 + 6 files changed, 99 insertions(+) create mode 100644 PowerEditor/src/MISC/Common/LongRunningOperation.cpp create mode 100644 PowerEditor/src/MISC/Common/LongRunningOperation.h diff --git a/PowerEditor/src/MISC/Common/LongRunningOperation.cpp b/PowerEditor/src/MISC/Common/LongRunningOperation.cpp new file mode 100644 index 00000000..27172f8f --- /dev/null +++ b/PowerEditor/src/MISC/Common/LongRunningOperation.cpp @@ -0,0 +1,42 @@ +// This file is part of Notepad++ project +// Copyright (C)2003 Don HO +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// Note that the GPL places important restrictions on "derived works", yet +// it does not provide a detailed definition of that term. To avoid +// misunderstandings, we consider an application to constitute a +// "derivative work" for the purpose of this license if it does any of the +// following: +// 1. Integrates source code from Notepad++. +// 2. Integrates/includes/aggregates Notepad++ into a proprietary executable +// installer, such as those produced by InstallShield. +// 3. Links to a library or executes a program that does any of the above. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + + +#include "LongRunningOperation.h" +#include + +std::recursive_mutex _operationMutex; + +LongRunningOperation::LongRunningOperation() +{ + _operationMutex.lock(); +} + +LongRunningOperation::~LongRunningOperation() +{ + _operationMutex.unlock(); +} \ No newline at end of file diff --git a/PowerEditor/src/MISC/Common/LongRunningOperation.h b/PowerEditor/src/MISC/Common/LongRunningOperation.h new file mode 100644 index 00000000..691f5b64 --- /dev/null +++ b/PowerEditor/src/MISC/Common/LongRunningOperation.h @@ -0,0 +1,39 @@ +// This file is part of Notepad++ project +// Copyright (C)2003 Don HO +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// Note that the GPL places important restrictions on "derived works", yet +// it does not provide a detailed definition of that term. To avoid +// misunderstandings, we consider an application to constitute a +// "derivative work" for the purpose of this license if it does any of the +// following: +// 1. Integrates source code from Notepad++. +// 2. Integrates/includes/aggregates Notepad++ into a proprietary executable +// installer, such as those produced by InstallShield. +// 3. Links to a library or executes a program that does any of the above. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + + +#ifndef M30_IDE_LONGRUNNINGOPERATION_h +#define M30_IDE_LONGRUNNINGOPERATION_h + +class LongRunningOperation +{ +public: + LongRunningOperation(); + ~LongRunningOperation(); +}; + +#endif //M30_IDE_LONGRUNNINGOPERATION_h diff --git a/PowerEditor/src/NppCommands.cpp b/PowerEditor/src/NppCommands.cpp index db16c461..b7ec67f1 100644 --- a/PowerEditor/src/NppCommands.cpp +++ b/PowerEditor/src/NppCommands.cpp @@ -36,6 +36,7 @@ #include "documentMap.h" #include "functionListPanel.h" #include "Sorters.h" +#include "LongRunningOperation.h" void Notepad_plus::macroPlayback(Macro macro) @@ -178,16 +179,22 @@ void Notepad_plus::command(int id) break; case IDM_EDIT_UNDO: + { + LongRunningOperation op; _pEditView->execute(WM_UNDO); checkClipboard(); checkUndoState(); break; + } case IDM_EDIT_REDO: + { + LongRunningOperation op; _pEditView->execute(SCI_REDO); checkClipboard(); checkUndoState(); break; + } case IDM_EDIT_CUT: _pEditView->execute(WM_CUT); @@ -354,6 +361,8 @@ void Notepad_plus::command(int id) case IDM_EDIT_SORTLINES_DECIMALDOT_ASCENDING: case IDM_EDIT_SORTLINES_DECIMALDOT_DESCENDING: { + LongRunningOperation op; + size_t fromLine = 0, toLine = 0; size_t fromColumn = 0, toColumn = 0; diff --git a/PowerEditor/src/ScitillaComponent/Buffer.cpp b/PowerEditor/src/ScitillaComponent/Buffer.cpp index a418be14..50dd54ac 100644 --- a/PowerEditor/src/ScitillaComponent/Buffer.cpp +++ b/PowerEditor/src/ScitillaComponent/Buffer.cpp @@ -34,6 +34,7 @@ #include "ScintillaEditView.h" #include "EncodingMapper.h" #include "uchardet.h" +#include "LongRunningOperation.h" FileManager * FileManager::_pSelf = new FileManager(); @@ -674,6 +675,8 @@ For untitled document (new 4) */ bool FileManager::backupCurrentBuffer() { + LongRunningOperation op; + Buffer * buffer = _pNotepadPlus->getCurrentBuffer(); bool result = false; bool hasModifForSession = false; diff --git a/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp b/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp index 63f86134..cd589f76 100644 --- a/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp +++ b/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp @@ -31,6 +31,7 @@ #include "ScintillaEditView.h" #include "Notepad_plus_msgs.h" #include "UniConversion.h" +#include "LongRunningOperation.h" FindOption * FindReplaceDlg::_env; FindOption FindReplaceDlg::_options; @@ -706,6 +707,7 @@ BOOL CALLBACK FindReplaceDlg::run_dlgProc(UINT message, WPARAM wParam, LPARAM lP case IDREPLACE : { + LongRunningOperation op; if (_currentStatus == REPLACE_DLG) { setStatusbarMessage(TEXT(""), FSNoMessage); @@ -792,6 +794,7 @@ BOOL CALLBACK FindReplaceDlg::run_dlgProc(UINT message, WPARAM wParam, LPARAM lP case IDD_FINDINFILES_REPLACEINFILES : { + LongRunningOperation op; setStatusbarMessage(TEXT(""), FSNoMessage); const int filterSize = 256; TCHAR filters[filterSize]; @@ -832,6 +835,7 @@ BOOL CALLBACK FindReplaceDlg::run_dlgProc(UINT message, WPARAM wParam, LPARAM lP case IDC_REPLACE_OPENEDFILES : { + LongRunningOperation op; if (_currentStatus == REPLACE_DLG) { setStatusbarMessage(TEXT(""), FSNoMessage); @@ -852,6 +856,7 @@ BOOL CALLBACK FindReplaceDlg::run_dlgProc(UINT message, WPARAM wParam, LPARAM lP case IDREPLACEALL : { + LongRunningOperation op; if (_currentStatus == REPLACE_DLG) { setStatusbarMessage(TEXT(""), FSNoMessage); diff --git a/PowerEditor/visual.net/notepadPlus.vcxproj b/PowerEditor/visual.net/notepadPlus.vcxproj index bab03030..762db800 100644 --- a/PowerEditor/visual.net/notepadPlus.vcxproj +++ b/PowerEditor/visual.net/notepadPlus.vcxproj @@ -138,6 +138,7 @@ copy ..\src\contextMenu.xml ..\bin\contextMenu.xml + From 5b28e27b3d467433083bd74096497b65b47ae469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20J=C3=B6nsson?= Date: Tue, 26 May 2015 22:54:11 +0200 Subject: [PATCH 2/3] Add "static" to get internal linkage. --- PowerEditor/src/MISC/Common/LongRunningOperation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PowerEditor/src/MISC/Common/LongRunningOperation.cpp b/PowerEditor/src/MISC/Common/LongRunningOperation.cpp index 27172f8f..7191197d 100644 --- a/PowerEditor/src/MISC/Common/LongRunningOperation.cpp +++ b/PowerEditor/src/MISC/Common/LongRunningOperation.cpp @@ -29,7 +29,7 @@ #include "LongRunningOperation.h" #include -std::recursive_mutex _operationMutex; +static std::recursive_mutex _operationMutex; LongRunningOperation::LongRunningOperation() { From 3ca488d0bc3d58b51dfc041d0ab378173313e4b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20J=C3=B6nsson?= Date: Sat, 30 May 2015 10:16:19 +0200 Subject: [PATCH 3/3] Also sync when pasting. See Sourceforge #5327. --- PowerEditor/src/NppCommands.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/PowerEditor/src/NppCommands.cpp b/PowerEditor/src/NppCommands.cpp index b7ec67f1..98542483 100644 --- a/PowerEditor/src/NppCommands.cpp +++ b/PowerEditor/src/NppCommands.cpp @@ -267,6 +267,7 @@ void Notepad_plus::command(int id) case IDM_EDIT_PASTE: { + LongRunningOperation op; int eolMode = int(_pEditView->execute(SCI_GETEOLMODE)); _pEditView->execute(SCI_PASTE); _pEditView->execute(SCI_CONVERTEOLS, eolMode); @@ -275,6 +276,7 @@ void Notepad_plus::command(int id) case IDM_EDIT_PASTE_BINARY: { + LongRunningOperation op; if (!IsClipboardFormatAvailable(CF_TEXT)) return; @@ -319,6 +321,7 @@ void Notepad_plus::command(int id) case IDM_EDIT_PASTE_AS_RTF: case IDM_EDIT_PASTE_AS_HTML: { + LongRunningOperation op; UINT f = RegisterClipboardFormat(id==IDM_EDIT_PASTE_AS_HTML?CF_HTML:CF_RTF); if (!IsClipboardFormatAvailable(f))