Skip to content

Commit cba6b6a

Browse files
committed
Always release the GIL when calling N++
Notifications can come at any time due to WM_NOTIFY messages from child windows being forwarded by N++ to plugins. This means we can always get a notification, even when we're not expecting it. Added check to see if we're interested in the notification before we claim the GIL. This is a belt-and-braces fix, as actually, simply performing a check if we're interested in the notification would be enough. However, there are so many hidden cases, always giving up the GIL whilst calling into N++ will eliminate all chance of deadlocks, and we can optimize later if it becomes an issue. Reported by "Kadner" https://sourceforge.net/p/npppythonscript/discussion/1188886/thread/915d6d40/?limit=25
1 parent f68c924 commit cba6b6a

2 files changed

Lines changed: 26 additions & 54 deletions

File tree

PythonScript/src/NotepadPlusWrapper.cpp

Lines changed: 23 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,25 @@ void NotepadPlusWrapper::notify(SCNotification *notifyCode)
4747
if (!m_notificationsEnabled)
4848
return;
4949

50+
51+
// Optimisation. Count the number of callbacks registered for this code,
52+
// if there are none, then we can simply return without claiming the GIL.
53+
// This is especially helpful as N++ forwards WM_NOTIFY messages from child windows, so we
54+
// get all manor of garbage from RebarWindows etc, that we just don't care about.
55+
// Overall, it's slightly slower in the case of a real callback (we count them, then
56+
// find them all with the equal_range() call below, but much quicker in the case of a
57+
// notification that we don't care about, as we don't need to grab the GIL then give it back.
58+
// We use count, because *ANY* operation that involves the boost::python::object (e.g. creating
59+
// an iterator) requires the GIL to manage the refcounts. A count() only involves the integer keys,
60+
// so we're safe to do that without the GIL.
61+
callbackT::size_type count = m_callbacks.count(notifyCode->nmhdr.code);
62+
if (0 == count)
63+
return;
64+
65+
DEBUG_TRACE_S(("Notepad notify with code %d\n", notifyCode->nmhdr.code));
66+
67+
68+
5069
GILLock gilLock;
5170

5271

@@ -165,7 +184,6 @@ bool NotepadPlusWrapper::addCallback(boost::python::object callback, boost::pyth
165184

166185
void NotepadPlusWrapper::save()
167186
{
168-
GILRelease release;
169187
callNotepad(NPPM_SAVECURRENTFILE);
170188

171189
}
@@ -176,7 +194,6 @@ void NotepadPlusWrapper::newDocument()
176194
DEBUG_TRACE(L"NotepadPlusWrapper::newDocument\n");
177195
notAllowedInScintillaCallback("new() cannot be called in a synchronous editor callback. "
178196
"Use an asynchronous callback, or avoid using new() in the callback handler");
179-
GILRelease release;
180197
callNotepad(NPPM_MENUCOMMAND, 0, IDM_FILE_NEW);
181198

182199
}
@@ -186,7 +203,6 @@ void NotepadPlusWrapper::newDocumentWithFilename(const char *filename)
186203
notAllowedInScintillaCallback("new() cannot be called in a synchronous editor callback. "
187204
"Use an asynchronous callback, or avoid using new() in the callback handler");
188205

189-
GILRelease release;
190206
callNotepad(NPPM_MENUCOMMAND, 0, IDM_FILE_NEW);
191207
std::shared_ptr<TCHAR> tFilename = WcharMbcsConverter::char2tchar(filename);
192208
callNotepad(NPPM_SAVECURRENTFILEAS, 0, reinterpret_cast<LPARAM>(tFilename.get()));
@@ -195,14 +211,12 @@ void NotepadPlusWrapper::newDocumentWithFilename(const char *filename)
195211

196212
void NotepadPlusWrapper::saveAs(const char *filename)
197213
{
198-
GILRelease release;
199214
callNotepad(NPPM_SAVECURRENTFILEAS, FALSE, reinterpret_cast<LPARAM>(WcharMbcsConverter::char2tchar(filename).get()));
200215

201216
}
202217

203218
void NotepadPlusWrapper::saveAsCopy(const char *filename)
204219
{
205-
GILRelease release;
206220
callNotepad(NPPM_SAVECURRENTFILEAS, TRUE, reinterpret_cast<LPARAM>(WcharMbcsConverter::char2tchar(filename).get()));
207221

208222
}
@@ -211,7 +225,6 @@ void NotepadPlusWrapper::open(const char *filename)
211225
{
212226
notAllowedInScintillaCallback("open() cannot be called in a synchronous editor callback. "
213227
"Use an asynchronous callback, or avoid using open() in the callback handler");
214-
GILRelease release;
215228
callNotepad(NPPM_DOOPEN, 0, reinterpret_cast<LPARAM>(WcharMbcsConverter::char2tchar(filename).get()));
216229

217230
}
@@ -221,7 +234,6 @@ bool NotepadPlusWrapper::activateFile(const char *filename)
221234
notAllowedInScintillaCallback("activateFile() cannot be called in a synchronous editor callback. "
222235
"Use an asynchronous callback, or avoid using activateFile() in the callback handler");
223236
bool retVal;
224-
GILRelease release;
225237
retVal = 0 != callNotepad(NPPM_SWITCHTOFILE, 0, reinterpret_cast<LPARAM>(WcharMbcsConverter::char2tchar(filename).get()));
226238

227239
return retVal;
@@ -244,7 +256,6 @@ LangType NotepadPlusWrapper::getCurrentLangType()
244256

245257
void NotepadPlusWrapper::setCurrentLangType(LangType lang)
246258
{
247-
GILRelease release;
248259
callNotepad(NPPM_SETCURRENTLANGTYPE, 0, static_cast<LPARAM>(lang));
249260

250261
}
@@ -369,7 +380,6 @@ void NotepadPlusWrapper::saveSession(const char *sessionFilename, boost::python:
369380

370381
si.nbFile = (int)filesCount;
371382

372-
GILRelease release;
373383
callNotepad(NPPM_SAVESESSION, 0, reinterpret_cast<LPARAM>(&si));
374384

375385

@@ -383,7 +393,6 @@ void NotepadPlusWrapper::saveSession(const char *sessionFilename, boost::python:
383393

384394
void NotepadPlusWrapper::saveCurrentSession(const char *filename)
385395
{
386-
GILRelease release;
387396
callNotepad(NPPM_SAVECURRENTSESSION, 0, reinterpret_cast<LPARAM>(WcharMbcsConverter::char2tchar(filename).get()));
388397

389398
}
@@ -412,7 +421,6 @@ idx_t NotepadPlusWrapper::getCurrentDocIndex(int view)
412421

413422
void NotepadPlusWrapper::setStatusBar(StatusBarSection section, const char *text)
414423
{
415-
GILRelease release;
416424
#ifdef UNICODE
417425
std::shared_ptr<TCHAR> s = WcharMbcsConverter::char2tchar(text);
418426
callNotepad(NPPM_SETSTATUSBAR, static_cast<WPARAM>(section), reinterpret_cast<LPARAM>(s.get()));
@@ -430,7 +438,6 @@ long NotepadPlusWrapper::getPluginMenuHandle()
430438

431439
void NotepadPlusWrapper::activateIndex(int view, int index)
432440
{
433-
GILRelease release;
434441
callNotepad(NPPM_ACTIVATEDOC, static_cast<WPARAM>(view), static_cast<LPARAM>(index));
435442

436443
}
@@ -441,10 +448,8 @@ void NotepadPlusWrapper::loadSession(boost::python::str filename)
441448
"Use an asynchronous callback, or avoid using loadSession() in the callback handler");
442449
#ifdef UNICODE
443450
std::shared_ptr<TCHAR> s = WcharMbcsConverter::char2tchar((const char*)boost::python::extract<const char*>(filename));
444-
GILRelease release;
445451
callNotepad(NPPM_LOADSESSION, 0, reinterpret_cast<LPARAM>(s.get()));
446452
#else
447-
GILRelease release;
448453
callNotepad(NPPM_LOADSESSION, 0, reinterpret_cast<LPARAM>((const char*)boost::python::extract<const char*>(filename)));
449454
#endif
450455

@@ -456,10 +461,8 @@ void NotepadPlusWrapper::activateFileString(boost::python::str filename)
456461
"Use an asynchronous callback, or avoid using activateFile() in the callback handler");
457462
#ifdef UNICODE
458463
std::shared_ptr<TCHAR> s = WcharMbcsConverter::char2tchar((const char*)boost::python::extract<const char*>(filename));
459-
GILRelease release;
460464
callNotepad(NPPM_SWITCHTOFILE, 0, reinterpret_cast<LPARAM>(s.get()));
461465
#else
462-
GILRelease release;
463466
callNotepad(NPPM_SWITCHTOFILE, 0, reinterpret_cast<LPARAM>((const char*)boost::python::extract<const char*>(filename)));
464467
#endif
465468

@@ -468,7 +471,6 @@ void NotepadPlusWrapper::activateFileString(boost::python::str filename)
468471

469472
void NotepadPlusWrapper::reloadFile(boost::python::str filename, bool alert)
470473
{
471-
GILRelease release;
472474
#ifdef UNICODE
473475
callNotepad(NPPM_RELOADFILE, alert ? 1 : 0, reinterpret_cast<LPARAM>(static_cast<const TCHAR *>(WcharMbcsConverter::char2tchar(boost::python::extract<const char *>(filename)).get())));
474476
#else
@@ -480,7 +482,6 @@ void NotepadPlusWrapper::reloadFile(boost::python::str filename, bool alert)
480482

481483
void NotepadPlusWrapper::saveAllFiles()
482484
{
483-
GILRelease release;
484485
callNotepad(NPPM_SAVEALLFILES);
485486

486487
}
@@ -494,7 +495,6 @@ boost::python::str NotepadPlusWrapper::getPluginConfigDir()
494495

495496
void NotepadPlusWrapper::menuCommand(int commandID)
496497
{
497-
GILRelease release;
498498
callNotepad(NPPM_MENUCOMMAND, 0, commandID);
499499

500500
}
@@ -542,101 +542,86 @@ boost::python::tuple NotepadPlusWrapper::getVersion()
542542

543543
void NotepadPlusWrapper::hideTabBar()
544544
{
545-
GILRelease release;
546545
callNotepad(NPPM_HIDETABBAR, 0, TRUE);
547546

548547
}
549548

550549
void NotepadPlusWrapper::showTabBar()
551550
{
552-
GILRelease release;
553551
callNotepad(NPPM_HIDETABBAR, 0, FALSE);
554552

555553
}
556554

557555
int NotepadPlusWrapper::getCurrentBufferID()
558556
{
559-
GILRelease release;
560557
return callNotepad(NPPM_GETCURRENTBUFFERID);
561558
}
562559

563560
void NotepadPlusWrapper::reloadBuffer(int bufferID, bool withAlert)
564561
{
565-
GILRelease release;
566562
callNotepad(NPPM_RELOADBUFFERID, static_cast<WPARAM>(bufferID), static_cast<LPARAM>(withAlert));
567563

568564
}
569565

570566
LangType NotepadPlusWrapper::getLangType()
571567
{
572-
GILRelease release;
573568
return getBufferLangType(callNotepad(NPPM_GETCURRENTBUFFERID));
574569
}
575570

576571
LangType NotepadPlusWrapper::getBufferLangType(int bufferID)
577572
{
578-
GILRelease release;
579573
return static_cast<LangType>(callNotepad(NPPM_GETBUFFERLANGTYPE, bufferID));
580574
}
581575

582576

583577

584578
void NotepadPlusWrapper::setBufferLangType(LangType language, int bufferID)
585579
{
586-
GILRelease release;
587580
callNotepad(NPPM_SETBUFFERLANGTYPE, static_cast<WPARAM>(bufferID), static_cast<LPARAM>(language));
588581

589582
}
590583

591584
BufferEncoding NotepadPlusWrapper::getEncoding()
592585
{
593-
GILRelease release;
594586
return getBufferEncoding(callNotepad(NPPM_GETCURRENTBUFFERID));
595587
}
596588

597589
BufferEncoding NotepadPlusWrapper::getBufferEncoding(int bufferID)
598590
{
599-
GILRelease release;
600591
return static_cast<BufferEncoding>(callNotepad(NPPM_GETBUFFERENCODING, static_cast<WPARAM>(bufferID)));
601592
}
602593

603594
void NotepadPlusWrapper::setEncoding(BufferEncoding encoding)
604595
{
605-
GILRelease release;
606596
callNotepad(NPPM_SETBUFFERENCODING, static_cast<WPARAM>(callNotepad(NPPM_GETCURRENTBUFFERID)), static_cast<LPARAM>(encoding));
607597

608598
}
609599

610600
void NotepadPlusWrapper::setBufferEncoding(BufferEncoding encoding, int bufferID)
611601
{
612-
GILRelease release;
613602
callNotepad(NPPM_SETBUFFERENCODING, static_cast<WPARAM>(bufferID), static_cast<LPARAM>(encoding));
614603

615604
}
616605

617606
FormatType NotepadPlusWrapper::getFormatType()
618607
{
619-
GILRelease release;
620608
return getBufferFormatType(callNotepad(NPPM_GETCURRENTBUFFERID));
621609
}
622610

623611

624612
FormatType NotepadPlusWrapper::getBufferFormatType(int bufferID)
625613
{
626-
GILRelease release;
627614
return static_cast<FormatType>(callNotepad(NPPM_GETBUFFERFORMAT, static_cast<WPARAM>(bufferID)));
628615
}
629616

630617
void NotepadPlusWrapper::setFormatType(FormatType format)
631618
{
632-
GILRelease release;
633619
setBufferFormatType(format, callNotepad(NPPM_GETCURRENTBUFFERID));
634620

635621
}
636622

637623
void NotepadPlusWrapper::setBufferFormatType(FormatType format, int bufferID)
638624
{
639-
GILRelease release;
640625
callNotepad(NPPM_SETBUFFERFORMAT, static_cast<WPARAM>(bufferID), static_cast<LPARAM>(format));
641626

642627
}
@@ -645,7 +630,6 @@ void NotepadPlusWrapper::closeDocument()
645630
{
646631
notAllowedInScintillaCallback("close() cannot be called in a synchronous editor callback. "
647632
"Use an asynchronous callback, or avoid using close() in the callback handler");
648-
GILRelease release;
649633
callNotepad(NPPM_MENUCOMMAND, 0, IDM_FILE_CLOSE);
650634

651635
}
@@ -654,7 +638,6 @@ void NotepadPlusWrapper::closeAllDocuments()
654638
{
655639
notAllowedInScintillaCallback("closeAll() cannot be called in a synchronous editor callback. "
656640
"Use an asynchronous callback, or avoid using closeAll() in the callback handler");
657-
GILRelease release;
658641
callNotepad(NPPM_MENUCOMMAND, 0, IDM_FILE_CLOSEALL);
659642

660643
}
@@ -663,14 +646,12 @@ void NotepadPlusWrapper::closeAllButCurrentDocument()
663646
{
664647
notAllowedInScintillaCallback("closeAllButCurrent() cannot be called in a synchronous editor callback. "
665648
"Use an asynchronous callback, or avoid using closeAllButCurrent() in the callback handler");
666-
GILRelease release;
667649
callNotepad(NPPM_MENUCOMMAND, 0, IDM_FILE_CLOSEALL_BUT_CURRENT);
668650

669651
}
670652

671653
void NotepadPlusWrapper::reloadCurrentDocument()
672654
{
673-
GILRelease release;
674655
callNotepad(NPPM_MENUCOMMAND, 0, IDM_FILE_RELOAD);
675656

676657
}
@@ -799,7 +780,6 @@ void NotepadPlusWrapper::activateBufferID(int bufferID)
799780
{
800781
notAllowedInScintillaCallback("activateBufferID() cannot be called in a synchronous editor callback. "
801782
"Use an asynchronous callback, or avoid using activateBufferID() in the callback handler");
802-
GILRelease release;
803783
idx_t index = (idx_t)callNotepad(NPPM_GETPOSFROMBUFFERID, static_cast<WPARAM>(bufferID));
804784
UINT view = (index & 0xC0000000) >> 30;
805785
index = index & 0x3FFFFFFF;
@@ -809,22 +789,18 @@ void NotepadPlusWrapper::activateBufferID(int bufferID)
809789
}
810790
boost::python::str NotepadPlusWrapper::getBufferFilename(int bufferID)
811791
{
812-
GILRelease release;
813792
TCHAR buffer[MAX_PATH];
814793
callNotepad(NPPM_GETFULLPATHFROMBUFFERID, static_cast<WPARAM>(bufferID), reinterpret_cast<LPARAM>(buffer));
815794
std::shared_ptr<char> filename = WcharMbcsConverter::tchar2char(buffer);
816-
release.reacquire();
817795
return boost::python::str(const_cast<const char *>(filename.get()));
818796
}
819797

820798
boost::python::str NotepadPlusWrapper::getCurrentFilename()
821799
{
822-
GILRelease release;
823800
idx_t bufferID = callNotepad(NPPM_GETCURRENTBUFFERID);
824801
TCHAR buffer[MAX_PATH];
825802
callNotepad(NPPM_GETFULLPATHFROMBUFFERID, bufferID, reinterpret_cast<LPARAM>(buffer));
826803
std::shared_ptr<char> filename = WcharMbcsConverter::tchar2char(buffer);
827-
release.reacquire();
828804
return boost::python::str(const_cast<const char *>(filename.get()));
829805
}
830806

@@ -876,9 +852,7 @@ bool NotepadPlusWrapper::runMenuCommand(boost::python::str menuName, boost::pyth
876852
boost::python::str NotepadPlusWrapper::getNppDir()
877853
{
878854
TCHAR buffer[MAX_PATH];
879-
GILRelease release;
880-
::SendMessage(m_nppHandle, NPPM_GETNPPDIRECTORY, MAX_PATH, reinterpret_cast<LPARAM>(buffer));
881-
release.reacquire();
855+
callNotepad(NPPM_GETNPPDIRECTORY, MAX_PATH, reinterpret_cast<LPARAM>(buffer));
882856
return boost::python::str(const_cast<const char *>(WcharMbcsConverter::tchar2char(buffer).get()));
883857
}
884858

@@ -889,16 +863,13 @@ boost::python::str NotepadPlusWrapper::getCommandLine()
889863

890864
bool NotepadPlusWrapper::allocateSupported()
891865
{
892-
GILRelease release;
893-
return 1 == ::SendMessage(m_nppHandle, NPPM_ALLOCATESUPPORTED, 0, 0);
866+
return 1 == callNotepad(NPPM_ALLOCATESUPPORTED);
894867
}
895868

896869
boost::python::object NotepadPlusWrapper::allocateCmdID(int quantity)
897870
{
898871
int startID;
899-
GILRelease release;
900-
bool result = 1 == ::SendMessage(m_nppHandle, NPPM_ALLOCATECMDID, static_cast<WPARAM>(quantity), reinterpret_cast<LPARAM>(&startID));
901-
release.reacquire();
872+
bool result = 1 == callNotepad(NPPM_ALLOCATECMDID, static_cast<WPARAM>(quantity), reinterpret_cast<LPARAM>(&startID));
902873
if (result)
903874
{
904875
return boost::python::object(startID);
@@ -912,9 +883,7 @@ boost::python::object NotepadPlusWrapper::allocateCmdID(int quantity)
912883
boost::python::object NotepadPlusWrapper::allocateMarker(int quantity)
913884
{
914885
int startID;
915-
GILRelease release;
916-
bool result = 1 == ::SendMessage(m_nppHandle, NPPM_ALLOCATEMARKER, static_cast<WPARAM>(quantity), reinterpret_cast<LPARAM>(&startID));
917-
release.reacquire();
886+
bool result = 1 == callNotepad(NPPM_ALLOCATEMARKER, static_cast<WPARAM>(quantity), reinterpret_cast<LPARAM>(&startID));
918887
if (result)
919888
{
920889
return boost::python::object(startID);

PythonScript/src/NotepadPlusWrapper.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include "menuCmdID.h"
1010
#endif
1111

12+
#include "GILManager.h"
13+
1214
struct SCNotification;
1315
namespace NppPythonScript
1416
{
@@ -575,6 +577,7 @@ class NotepadPlusWrapper
575577
protected:
576578
LRESULT callNotepad(UINT message, WPARAM wParam = 0, LPARAM lParam = 0)
577579
{
580+
GILRelease release;
578581
return SendMessage(m_nppHandle, message, wParam, lParam);
579582
}
580583

0 commit comments

Comments
 (0)