Avasam
commented
6 months ago Tagging @sschukat as the original SourceForge issue creator.
Proposed patch with updated formatting and location:
diff --git a/com/win32com/src/PyGatewayBase.cpp b/com/win32com/src/PyGatewayBase.cpp
index 83618d68..cf4e0a5a 100644
--- a/com/win32com/src/PyGatewayBase.cpp
+++ b/com/win32com/src/PyGatewayBase.cpp
@@ -90,8 +90,8 @@ PyGatewayBase::PyGatewayBase(PyObject *instance)
m_cRef = 1;
m_pPyObject = instance;
Py_XINCREF(instance); // instance should never be NULL - but whats an X between friends!
-
- PyCom_DLLAddRef();
+ Py_BEGIN_ALLOW_THREADS PyCom_DLLAddRef();
+ Py_END_ALLOW_THREADS
#ifdef DEBUG_FULL
PyCom_LogF(U"PyGatewayBase: created %s", m_pPyObject ? m_pPyObject->ob_type->tp_name : "<NULL>");
@@ -105,7 +105,10 @@ PyGatewayBase::~PyGatewayBase()
PyCom_LogF(L"PyGatewayBase: deleted %s", m_pPyObject ? m_pPyObject->ob_type->tp_name : "<NULL>");
#endif
- if (m_pPyObject) {
+ // Check if the Python Runtime is still alive when a COM Objekt shall be released.
+ // It could happen that late in the process lifetime the object shall be released and Python
+ // was already shut down.
+ if (m_pPyObject && 0 != Py_IsInitialized()) {
{
CEnterLeavePython celp;
Py_DECREF(m_pPyObject);
diff --git a/com/win32com/src/PyIClassFactory.cpp b/com/win32com/src/PyIClassFactory.cpp
index 649aa4f8..e38f5ad0 100644
--- a/com/win32com/src/PyIClassFactory.cpp
+++ b/com/win32com/src/PyIClassFactory.cpp
@@ -10,14 +10,16 @@ PyIClassFactory::PyIClassFactory(IUnknown *pDisp) : PyIUnknown(pDisp)
ob_type = &type;
// Class Factory interfaces do not count towards DLL Ref counts,
// but the PyIUnknown ctor Added a reference.
- PyCom_DLLReleaseRef();
+ Py_BEGIN_ALLOW_THREADS PyCom_DLLReleaseRef();
+ Py_END_ALLOW_THREADS
}
PyIClassFactory::~PyIClassFactory()
{
// Class Factory interfaces do not count towards DLL Ref counts,
// but the PyIUnknown dtor Releases a reference.
- PyCom_DLLAddRef();
+ Py_BEGIN_ALLOW_THREADS PyCom_DLLAddRef();
+ Py_END_ALLOW_THREADS
}
/*static*/ IClassFactory *PyIClassFactory::GetI(PyObject *self) { return (IClassFactory *)PyIUnknown::GetI(self); }
diff --git a/com/win32com/src/PyIUnknown.cpp b/com/win32com/src/PyIUnknown.cpp
index 1473cb8a..e654b975 100644
--- a/com/win32com/src/PyIUnknown.cpp
+++ b/com/win32com/src/PyIUnknown.cpp
@@ -17,14 +17,16 @@ PyIUnknown::PyIUnknown(IUnknown *punk)
m_obj = punk;
// refcnt of object managed by caller.
InterlockedIncrement(&cUnknowns);
- PyCom_DLLAddRef();
+ Py_BEGIN_ALLOW_THREADS PyCom_DLLAddRef();
+ Py_END_ALLOW_THREADS
}
PyIUnknown::~PyIUnknown()
{
SafeRelease(this);
InterlockedDecrement(&cUnknowns);
- PyCom_DLLReleaseRef();
+ Py_BEGIN_ALLOW_THREADS PyCom_DLLReleaseRef();
+ Py_END_ALLOW_THREADS
}
// @method string|PyIUnknown|__repr__|Called to create a representation of a PyIUnknown object
PyObject *PyIUnknown::repr()
diff --git a/com/win32com/src/dllmain.cpp b/com/win32com/src/dllmain.cpp
index 5cbfbba3..79521d21 100644
--- a/com/win32com/src/dllmain.cpp
+++ b/com/win32com/src/dllmain.cpp
@@ -173,7 +173,8 @@ typedef HRESULT(WINAPI *PFNCoInitializeEx)(LPVOID pvReserved, DWORD dwCoInit);
HRESULT PyCom_CoInitializeEx(LPVOID reserved, DWORD dwInit)
{
// Must be thread-safe, although doesnt need the Python lock.
- CEnterLeaveFramework _celf;
+ PyWin_AcquireGlobalLock();
+ // CEnterLeaveFramework _celf;
if (g_bCoInitThreadHasInit && g_dwCoInitThread == GetCurrentThreadId())
return S_OK;
#ifndef MS_WINCE
@@ -245,12 +246,16 @@ void PyCom_CoUninitialize()
// being asked to terminate on our "main" thread
// Check our flag, but always consider it success.
if (g_bCoInitThreadHasInit) {
- CoUninitialize();
g_bCoInitThreadHasInit = FALSE;
+ // Release the lock since CoInitialize could synchronize
+ // with another STA thread.
+ PyWin_ReleaseGlobalLock();
+ CoUninitialize();
}
}
else {
// Not our thread - assume caller knows what they are doing
+ PyWin_ReleaseGlobalLock();
CoUninitialize();
}
}This overlaps with #638
If running in a multithreaded environment deadlocks could occur due to the fact that two locks are used (GIL and PythonCOM lock).
E.g. three thread scenario: First thread has the GIL and tries to get an interface from the GIT.
Callstack: ... 18f5e798 09ea9b8b 09eb62d4 09e4696c 09e88c08 ntdll!RtlEnterCriticalSection+0x150 18f5e7a0 09e4696c 09e88c08 18f5e7c4 09e64228 pywintypes25!PyWin_AcquireGlobalLock+0xb 18f5e7b4 09e3879e 13828118 13828118 18f5e7e8 pythoncom25!PyCom_DLLAddRef+0x1c 18f5e7cc 09e371fd 0c718804 0c718804 09e3139b pythoncom25!PyIUnknown::PyIUnknown+0x4e 18f5e7d8 09e3139b 0c718804 13828118 18f5ff78 pythoncom25!PyIDispatch::PyIDispatch+0xd 18f5e7f0 09e345d2 0c718804 12940418 00000000 pythoncom25!PyIDispatch::PyObConstruct+0x3b 18f5e804 09e53024 0c718804 18f5e830 00000000 pythoncom25!PyCom_PyObjectFromIUnknown+0xc2 18f5e83c 09c4e317 0a0ed3bc 16c09f58 18f5e8ac pythoncom25!PyIGlobalInterfaceTable::GetInterfaceFromGlobal+0xb4
To achieve this it needs the PythonCOM lock.
A second thread has the PythonCOM lock while calling CoUninitialize which wants to release a COM interface in the main thread (ole32!RemoteReleaseRifRef and ole32!SwitchSTA).
Callstack: … 187af0e0 76f7a7c9 00a6b360 187af1d8 187af1ec ole32!SwitchSTA+0x21 .. 187af7ac 76e7b172 0c711104 00a6b360 00000001 ole32!RemoteReleaseRifRef+0xb0 … 187afad4 09e46ce5 09e3edae 0a1f4828 09c4e317 ole32!CoUninitialize+0x72 187afad8 09e3edae 0a1f4828 09c4e317 00000000 pythoncom25!PyCom_CoUninitialize+0x35
The main thread wants to execute Python code and waits for the GIL and hence blocking the message loop.
Callstack. … 0018f850 09c8e0c7 0a0e6d78 ffffffff 09bf8f28 python25!EnterNonRecursiveMutex+0x3d 0018f85c 09bf8f28 0a0e6d78 00000001 00000000 python25!PyThread_acquire_lock+0x17 0018f874 09c63f1c 0a0e24c0 ffffffff 0261f870 python25!PyEval_RestoreThread+0x38 0018f884 09b85588 0261f870 099b58fc 02642d10 python25!PyGILState_Ensure+0x5c
So first thread holds the GIL while waiting for the PythonCOM lock. Second thread holds the PythonCOM lock while synchronizing with the main thread. The main thread waits for the GIL, i.e. deadlock.
Two thread scenario. Almost as above only exception the GIL is not involved. A second thread wants to call CoUninitialize while holding the PythonCOM lock triggers a synchronization with the main thread due to the STA model. The main thread blocks the message loop since it waits for the PythonCOM lock while calling PyCom_DLLAddRef.
A attach a patch which is based on following ideas.
With these changes the deadlock did not occur anymore in our product.
Reported by: sschukat
Original Ticket: pywin32/bugs/649