[PATCH v3 0/2] MR3372: ole32: Leave the RunningObjectTable Critical Section before unmarshalling object stream

[Lorenzo Ferrillo (＠llde)]

In certain circumstances unmarshalling an object stream from the RunningObjectTable can cause the unmarshalling routines to interrogate the same 
table (maybe to resolve a dependant object?) in a different thread causing a deadlock while getting the critical section lock. Leaving the Critical Section before unmarshalling the object stream solve this problem.  
Visual Studio 2019 deadlock on start without this.

I'm not sure how to test this properly.
Also add debug info for this critical section so it show a significant name in the log.

Signed-off-by: Lorenzo Ferrillo

--
  v3: ole32: Add debug info to RunningObjectTable critical section

https://gitlab.winehq.org/wine/wine/-/merge_requests/3372

[PATCH v3 1/2] ole32: Leave the RunningObjectTable Critical Section before umarshalling object

[Lorenzo Ferrillo]

From: Lorenzo Ferrillo

Sometimes umarshalling an object from the RunningObjectTable can cause the routine to access the
table back inside another thread, causing a deadlock.
Visual Studio 2019 is an example of this behaviour.

Signed-off-by: Lorenzo Ferrillo 
---
 dlls/ole32/moniker.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/ole32/moniker.c b/dlls/ole32/moniker.c
index ec026758b2f..06a2ef8de9c 100644
--- a/dlls/ole32/moniker.c
+++ b/dlls/ole32/moniker.c
@@ -541,13 +541,14 @@ RunningObjectTableImpl_GetObject( IRunningObjectTable* iface,
         {
             IStream *pStream;
             hr = create_stream_on_mip_ro(rot_entry->object, &pStream);
+            LeaveCriticalSection(&This->lock);
+            
             if (hr == S_OK)
             {
                 hr = CoUnmarshalInterface(pStream, &IID_IUnknown, (void **)ppunkObject);
                 IStream_Release(pStream);
             }

-            LeaveCriticalSection(&This->lock);
             HeapFree(GetProcessHeap(), 0, moniker_data);

return hr;

[PATCH v3 2/2] ole32: Add debug info to RunningObjectTable critical section

[Lorenzo Ferrillo]

From: Lorenzo Ferrillo

So it's possible to easily debug deadlocks involving the Internal ROT object

Signed-off-by: Lorenzo Ferrillo 
---
 dlls/ole32/moniker.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/dlls/ole32/moniker.c b/dlls/ole32/moniker.c
index 06a2ef8de9c..2e55516b6f0 100644
--- a/dlls/ole32/moniker.c
+++ b/dlls/ole32/moniker.c
@@ -707,10 +707,19 @@ static const IRunningObjectTableVtbl VT_RunningObjectTableImpl =
     RunningObjectTableImpl_EnumRunning
 };

+static RunningObjectTableImpl rot;
+
+static RTL_CRITICAL_SECTION_DEBUG critsect_debug =
+{
+    0, 0, &rot.lock,
+    { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList },
+      0, 0, { (DWORD_PTR)(__FILE__ ": RunningObjectTable_section") }
+};
+
 static RunningObjectTableImpl rot =
 {
     .IRunningObjectTable_iface.lpVtbl = &VT_RunningObjectTableImpl,
-    .lock.LockCount = -1,
+    .lock = { &critsect_debug, -1, 0, 0, 0, 0 },
     .rot = LIST_INIT(rot.rot),
 };

[Lorenzo Ferrillo (＠llde)]

Hi @tmatthies I rebased with last master and was able to add the debug section without changing the struct to a pointer. Tell me if you prefers this version

[Torge Matthies (＠tmatthies)]

On Sun Sep 10 16:36:33 2023 +0000, Lorenzo Ferrillo wrote:
> Hi @tmatthies I rebased with last master and was able to add the debug
> section without changing the struct to a pointer. Tell me if you prefers
> this version

Yeah that's a much smaller change, I prefer that

[Zebediah Figura (＠zfigura)]

> This looks fine to me, except for the first commit. Even if our ole32 is unlikely to run on Windows, using this customization is not great. There might be other opinions.

It's done elsewhere, e.g. in wined3d, which does consistently work on Windows. Which is not to offer an argument, but at least an observation.