Skip to content

Commit e2e51c2

Browse files
brudersteinbruderstein
committed
CallbackExecArgs deallocation bug
CallbackExecArgs contains boost python objects, and was deallocated with a shared_ptr. Unfortunately this wasn't always when we still had the GIL. Changed to pointers, then we explicitly grab the GIL when in the destructor. CallbackExecArgs is now in the namespace NppPythonScript - many other classes also removed from global namespace. There's more to do in this area though.
1 parent 111c7d0 commit e2e51c2

18 files changed

Lines changed: 840 additions & 724 deletions

‎PythonScript.Tests/PythonScript.Tests.vcxproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
<PrecompiledHeader>Use</PrecompiledHeader>
5555
<WarningLevel>Level3</WarningLevel>
5656
<Optimization>Disabled</Optimization>
57-
<PreprocessorDefinitions>WIN32;_DEBUG;_CONSOLE;BOOST_PYTHON_STATIC_LIB;%(PreprocessorDefinitions)</PreprocessorDefinitions>
57+
<PreprocessorDefinitions>WIN32;BOOST_DEBUG_PYTHON;BOOST_LINKING_PYTHON;_DEBUG;_CONSOLE;BOOST_PYTHON_STATIC_LIB;%(PreprocessorDefinitions)</PreprocessorDefinitions>
5858
<AdditionalIncludeDirectories>gtest\include;..\PythonScript\src;$(BoostBase);%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
5959
<RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
6060
</ClCompile>

‎PythonScript/project/PythonScript2010.vcxproj‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@
118118
<PrecompiledHeader>Use</PrecompiledHeader>
119119
<WarningLevel>Level4</WarningLevel>
120120
<Optimization>Disabled</Optimization>
121-
<PreprocessorDefinitions>WIN32;_DEBUG;_WINDOWS;_USRDLL;PYTHONSCRIPT2010_EXPORTS;BOOST_PYTHON_STATIC_LIB;%(PreprocessorDefinitions)</PreprocessorDefinitions>
121+
<PreprocessorDefinitions>WIN32;BOOST_DEBUG_PYTHON;BOOST_LINKING_PYTHON;_DEBUG;_WINDOWS;_USRDLL;PYTHONSCRIPT2010_EXPORTS;BOOST_PYTHON_STATIC_LIB;%(PreprocessorDefinitions)</PreprocessorDefinitions>
122122
<RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
123123
<AdditionalIncludeDirectories>../include;../res;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
124124
<MultiProcessorCompilation>true</MultiProcessorCompilation>
@@ -127,7 +127,7 @@
127127
<Link>
128128
<SubSystem>Windows</SubSystem>
129129
<GenerateDebugInformation>true</GenerateDebugInformation>
130-
<AdditionalDependencies>python27.lib;NppPlugin.lib;shlwapi.lib;comctl32.lib;htmlhelp.lib;%(AdditionalDependencies)</AdditionalDependencies>
130+
<AdditionalDependencies>NppPlugin.lib;shlwapi.lib;comctl32.lib;htmlhelp.lib;dbghelp.lib;%(AdditionalDependencies)</AdditionalDependencies>
131131
<AdditionalLibraryDirectories>$(HtmlHelpBase)\lib;$(BoostLibPath)</AdditionalLibraryDirectories>
132132
<ShowProgress>LinkVerbose</ShowProgress>
133133
</Link>
@@ -270,6 +270,7 @@ xcopy $(ProjectDir)..\python_tests\*.* "e:\notepadtest\unicode\plugins\config\py
270270
<ItemGroup>
271271
<ClCompile Include="..\src\AboutDialog2.cpp" />
272272
<ClCompile Include="..\src\ArgumentException.cpp" />
273+
<ClCompile Include="..\src\CallbackExecArgs.cpp" />
273274
<ClCompile Include="..\src\ConfigFile.cpp" />
274275
<ClCompile Include="..\src\ConsoleDialog.cpp" />
275276
<ClCompile Include="..\src\DebugTrace.cpp" />
@@ -314,6 +315,7 @@ xcopy $(ProjectDir)..\python_tests\*.* "e:\notepadtest\unicode\plugins\config\py
314315
<ClInclude Include="..\src\AboutDialog.h" />
315316
<ClInclude Include="..\src\ANSIIterator.h" />
316317
<ClInclude Include="..\src\ArgumentException.h" />
318+
<ClInclude Include="..\src\CallbackExecArgs.h" />
317319
<ClInclude Include="..\src\ConfigFile.h" />
318320
<ClInclude Include="..\src\ConsoleDialog.h" />
319321
<ClInclude Include="..\src\ConsoleInterface.h" />

‎PythonScript/project/PythonScript2010.vcxproj.filters‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,9 @@
154154
<ClCompile Include="..\src\DebugTrace.cpp">
155155
<Filter>Source Files</Filter>
156156
</ClCompile>
157+
<ClCompile Include="..\src\CallbackExecArgs.cpp">
158+
<Filter>Source Files</Filter>
159+
</ClCompile>
157160
</ItemGroup>
158161
<ItemGroup>
159162
<ClInclude Include="..\src\AboutDialog.h">
@@ -339,6 +342,9 @@
339342
<ClInclude Include="..\src\GILManager.h">
340343
<Filter>Header Files</Filter>
341344
</ClInclude>
345+
<ClInclude Include="..\src\CallbackExecArgs.h">
346+
<Filter>Header Files</Filter>
347+
</ClInclude>
342348
</ItemGroup>
343349
<ItemGroup>
344350
<ResourceCompile Include="..\res\PythonScript.rc">

‎PythonScript/src/CallbackExecArgs.cpp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#include "stdafx.h"
2+
#include "CallbackExecArgs.h"
3+
#include "GILManager.h"
4+
5+
6+
namespace NppPythonScript
7+
{
8+
9+
CallbackExecArgs::~CallbackExecArgs()
10+
{
11+
GILLock gilLock = GILManager::getGIL();
12+
delete m_callbacks;
13+
if (NULL != m_params)
14+
{
15+
delete m_params;
16+
}
17+
}
18+
19+
20+
void CallbackExecArgs::setParams(boost::python::dict params)
21+
{
22+
if (NULL != params)
23+
{
24+
delete m_params;
25+
}
26+
m_params = new boost::python::dict(params);
27+
}
28+
}

‎PythonScript/src/CallbackExecArgs.h‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#ifndef CALLBACKEXECARGS_20140217_H
2+
#define CALLBACKEXECARGS_20140217_H
3+
4+
5+
namespace NppPythonScript
6+
{
7+
8+
class CallbackExecArgs
9+
{
10+
public:
11+
CallbackExecArgs()
12+
: m_callbacks(new std::list<boost::python::object>()),
13+
m_params(NULL)
14+
{}
15+
16+
virtual ~CallbackExecArgs();
17+
18+
void addCallback(boost::python::object callback) { m_callbacks->push_back(callback); }
19+
void setParams(boost::python::dict params);
20+
21+
std::list<boost::python::object> *getCallbacks() { return m_callbacks; }
22+
boost::python::dict *getParams() { return m_params; }
23+
private:
24+
std::list<boost::python::object> *m_callbacks;
25+
boost::python::dict *m_params;
26+
};
27+
28+
}
29+
30+
#endif // CALLBACKEXECARGS_20140217_H

‎PythonScript/src/NotepadPlusWrapper.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ void NotepadPlusWrapper::save()
166166

167167
void NotepadPlusWrapper::newDocument()
168168
{
169+
DEBUG_TRACE(L"NotepadPlusWrapper::newDocument - releasing GIL");
169170
NppPythonScript::GILRelease release = NppPythonScript::GILManager::releaseGIL();
170171
callNotepad(NPPM_MENUCOMMAND, 0, IDM_FILE_NEW);
171172

@@ -371,15 +372,15 @@ void NotepadPlusWrapper::saveCurrentSession(const char *filename)
371372

372373
}
373374

374-
ScintillaWrapper NotepadPlusWrapper::createScintilla()
375+
NppPythonScript::ScintillaWrapper NotepadPlusWrapper::createScintilla()
375376
{
376377
LRESULT handle = callNotepad(NPPM_CREATESCINTILLAHANDLE, 0, NULL);
377378

378379
// return copy
379-
return ScintillaWrapper((HWND)handle, m_nppHandle);
380+
return NppPythonScript::ScintillaWrapper((HWND)handle, m_nppHandle);
380381
}
381382

382-
void NotepadPlusWrapper::destroyScintilla(ScintillaWrapper& buffer)
383+
void NotepadPlusWrapper::destroyScintilla(NppPythonScript::ScintillaWrapper& buffer)
383384
{
384385
callNotepad(NPPM_DESTROYSCINTILLAHANDLE, 0, reinterpret_cast<LPARAM>(buffer.getHandle()));
385386
buffer.invalidateHandle();

‎PythonScript/src/NotepadPlusWrapper.h‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010
#endif
1111

1212
struct SCNotification;
13+
namespace NppPythonScript
14+
{
1315
class ScintillaWrapper;
16+
}
17+
1418

1519
enum FormatType
1620
{
@@ -421,7 +425,10 @@ enum MenuCommands
421425
NPPIDM_SYSTRAYPOPUP_CLOSE = IDM_SYSTRAYPOPUP_CLOSE
422426
};
423427

424-
struct CallbackExecArgs;
428+
namespace NppPythonScript
429+
{
430+
class CallbackExecArgs;
431+
}
425432

426433
class NotepadPlusWrapper
427434
{
@@ -454,8 +461,8 @@ class NotepadPlusWrapper
454461
void saveSession(const char *sessionFilename, boost::python::list files);
455462
void saveCurrentSession(const char *filename);
456463

457-
ScintillaWrapper createScintilla();
458-
void destroyScintilla(ScintillaWrapper& buffer);
464+
NppPythonScript::ScintillaWrapper createScintilla();
465+
void destroyScintilla(NppPythonScript::ScintillaWrapper& buffer);
459466

460467

461468
idx_t getCurrentDocIndex(int view);
@@ -585,7 +592,7 @@ class NotepadPlusWrapper
585592
callbackT m_callbacks;
586593
bool m_notificationsEnabled;
587594

588-
static void runCallbacks(CallbackExecArgs *args);
595+
static void runCallbacks(NppPythonScript::CallbackExecArgs *args);
589596
};
590597

591598

‎PythonScript/src/PythonConsole.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
#include "NotepadPlusWrapper.h"
1616

1717
PythonConsole::PythonConsole(HWND hNotepad) :
18-
mp_scintillaWrapper(new ScintillaWrapper(NULL, hNotepad)),
18+
mp_scintillaWrapper(new NppPythonScript::ScintillaWrapper(NULL, hNotepad)),
1919
mp_consoleDlg(new ConsoleDialog()),
2020
mp_mainThreadState(NULL),
2121
m_statementRunning(CreateEvent(NULL, FALSE, TRUE, NULL)),
@@ -31,7 +31,7 @@ PythonConsole::PythonConsole(HWND hNotepad) :
3131
PythonConsole::PythonConsole(const PythonConsole& other) :
3232
NppPythonScript::PyProducerConsumer<std::string>(other),
3333
ConsoleInterface(other),
34-
mp_scintillaWrapper(other.mp_scintillaWrapper ? new ScintillaWrapper(*other.mp_scintillaWrapper) : NULL),
34+
mp_scintillaWrapper(other.mp_scintillaWrapper ? new NppPythonScript::ScintillaWrapper(*other.mp_scintillaWrapper) : NULL),
3535
mp_consoleDlg(other.mp_consoleDlg ? new ConsoleDialog(*other.mp_consoleDlg) : NULL),
3636
m_console(other.m_console),
3737
m_pushFunc(other.m_pushFunc),
@@ -82,7 +82,7 @@ void PythonConsole::init(HINSTANCE hInst, NppData& nppData)
8282
}
8383
}
8484

85-
void PythonConsole::initPython(PythonHandler *pythonHandler)
85+
void PythonConsole::initPython(NppPythonScript::PythonHandler *pythonHandler)
8686
{
8787
try
8888
{
@@ -277,7 +277,7 @@ void PythonConsole::consume(std::shared_ptr<std::string> statement)
277277
m_sys.attr("stdout") = boost::python::ptr(this);
278278
PyObject* unicodeCommand = PyUnicode_FromEncodedObject(boost::python::str(statement->c_str()).ptr(), "utf-8", NULL);
279279
boost::python::object result = m_pushFunc(boost::python::handle<PyObject>(unicodeCommand));
280-
Py_DECREF(unicodeCommand);
280+
//Py_DECREF(unicodeCommand);
281281
m_sys.attr("stdout") = oldStdout;
282282

283283
continuePrompt = boost::python::extract<bool>(result);

‎PythonScript/src/PythonConsole.h‎

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

12+
namespace NppPythonScript
13+
{
1214
class PythonHandler;
1315
class ScintillaWrapper;
16+
}
17+
1418
class ConsoleDialog;
1519
struct RunStatementArgs;
1620
struct NppData;
@@ -23,7 +27,7 @@ class PythonConsole : public NppPythonScript::PyProducerConsumer<std::string>, p
2327
~PythonConsole();
2428

2529
void init(HINSTANCE hInst, NppData& nppData);
26-
void initPython(PythonHandler *pythonHandler);
30+
void initPython(NppPythonScript::PythonHandler *pythonHandler);
2731

2832
void showDialog();
2933
// Show console but fire a message to get it created on the right thread.
@@ -63,9 +67,9 @@ class PythonConsole : public NppPythonScript::PyProducerConsumer<std::string>, p
6367

6468
HWND getScintillaHwnd();
6569

66-
ScintillaWrapper& getScintillaWrapper() { return *mp_scintillaWrapper; }
70+
NppPythonScript::ScintillaWrapper& getScintillaWrapper() { return *mp_scintillaWrapper; }
6771

68-
ScintillaWrapper* mp_scintillaWrapper;
72+
NppPythonScript::ScintillaWrapper* mp_scintillaWrapper;
6973

7074
static boost::python::str getEncoding() { return boost::python::str("utf-8"); }
7175

‎PythonScript/src/PythonHandler.cpp‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
#include "WcharMbcsConverter.h"
1212
#include "GILManager.h"
1313

14+
namespace NppPythonScript
15+
{
16+
1417
PythonHandler::PythonHandler(TCHAR *pluginsDir, TCHAR *configDir, HINSTANCE hInst, HWND nppHandle, HWND scintilla1Handle, HWND scintilla2Handle, PythonConsole *pythonConsole)
1518
: PyProducerConsumer<RunScriptArgs>(),
1619
m_nppHandle(nppHandle),
@@ -72,10 +75,11 @@ PythonHandler::~PythonHandler(void)
7275
}
7376
}
7477

75-
ScintillaWrapper* PythonHandler::createScintillaWrapper()
78+
79+
NppPythonScript::ScintillaWrapper* PythonHandler::createScintillaWrapper()
7680
{
7781
m_currentView = mp_notepad->getCurrentView();
78-
return new ScintillaWrapper(m_currentView ? m_scintilla2Handle : m_scintilla1Handle, m_nppHandle);
82+
return new NppPythonScript::ScintillaWrapper(m_currentView ? m_scintilla2Handle : m_scintilla1Handle, m_nppHandle);
7983
}
8084

8185
NotepadPlusWrapper* PythonHandler::createNotepadPlusWrapper()
@@ -320,3 +324,6 @@ void PythonHandler::stopScriptWorker(PythonHandler *handler)
320324
PyThreadState_SetAsyncExc((long)handler->getExecutingThreadID(), PyExc_KeyboardInterrupt);
321325

322326
}
327+
328+
329+
}

0 commit comments

Comments
 (0)