include/systools/win32/wait_for_multiple_objects.hxx | 37 +++++++++++++++++++
sal/osl/w32/conditn.cxx | 32 ++++------------
vcl/win/dtrans/MtaOleClipb.cxx | 30 +--------------
3 files changed, 49 insertions(+), 50 deletions(-)
New commits:
commit d86d2dc58716d3605f0d8b09623400c57c3d9809
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Fri Jan 10 16:19:10 2025 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Mon Feb 17 16:34:18 2025 +0500
Factor out and simplify COM-safe wait
Before commit 1e2e51607a163021ef1fb1fb0d217266bd448173 (Try to use
CoWaitForMultipleHandles magic to handle COM message loop, 2025-01-10)
osl_waitCondition didn't adjust the timeout value in the COM message
processing loop. The abovementioned commit started to use a function
that pumps the message loop itself, but the code for manual pumping
wasn't removed (it has (likely) became dead).
This drops the dead code, and deduplicates the rest.
Change-Id: I0f7bbf8220792fbe42178c2689a065b5f38645d6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/180069
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/include/systools/win32/wait_for_multiple_objects.hxx
b/include/systools/win32/wait_for_multiple_objects.hxx
new file mode 100644
index 000000000000..bcffe7013d8b
--- /dev/null
+++ b/include/systools/win32/wait_for_multiple_objects.hxx
@@ -0,0 +1,37 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4;
fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#pragma once
+
+#include <sal/config.h>
+
+#include <prewin.h>
+#include <objbase.h>
+#include <postwin.h>
+
+namespace sal::systools
+{
+/* Like WaitForMultipleObjects function, but makes sure to process COM
messages during the wait,
+ * allowing other threads access to COM objects instantiated in this thread.
+ */
+DWORD WaitForMultipleObjects_COMDispatch(DWORD count, const HANDLE* handles,
DWORD timeout)
+{
+ DWORD dwResult;
+ HRESULT hr = CoWaitForMultipleHandles(COWAIT_DISPATCH_CALLS, timeout,
count,
+ const_cast<LPHANDLE>(handles),
&dwResult);
+ if (hr == RPC_S_CALLPENDING)
+ return WAIT_TIMEOUT;
+ if (FAILED(hr))
+ return WAIT_FAILED;
+
+ return dwResult;
+}
+} // sal::systools
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
diff --git a/sal/osl/w32/conditn.cxx b/sal/osl/w32/conditn.cxx
index 0ad233ed9bff..9063899533c3 100644
--- a/sal/osl/w32/conditn.cxx
+++ b/sal/osl/w32/conditn.cxx
@@ -22,6 +22,7 @@
#include <osl/conditn.h>
#include <osl/diagnose.h>
#include <osl/time.h>
+#include <systools/win32/wait_for_multiple_objects.hxx>
/*
under WIN32, we use the void* oslCondition
@@ -75,39 +76,17 @@ oslConditionResult SAL_CALL osl_waitCondition(oslCondition
Condition,
/* It's necessary to process SendMessage calls to the current thread to
give other threads
access to COM objects instantiated in this thread */
-
- while ( true )
+ switch (sal::systools::WaitForMultipleObjects_COMDispatch(
+ 1, reinterpret_cast<HANDLE*>(&Condition), timeout))
{
- DWORD index;
- /* Only wake up if a SendMessage / COM call to the threads message
loop is detected */
- HRESULT hr = CoWaitForMultipleHandles(COWAIT_DISPATCH_CALLS, timeout,
1,
-
reinterpret_cast<HANDLE*>(&Condition), &index);
- if (hr == RPC_S_CALLPENDING)
- return osl_cond_result_timeout;
- if (FAILED(hr))
- return osl_cond_result_error;
- switch (index)
- {
- case WAIT_OBJECT_0 + 1:
- {
- MSG msg;
-
- /* We Must not dispatch the message. PM_NOREMOVE leaves the
message queue untouched
- but dispatches SendMessage calls automatically */
+ case WAIT_OBJECT_0:
+ return osl_cond_result_ok;
- PeekMessageW( &msg, nullptr, 0, 0, PM_NOREMOVE );
- }
- break;
-
- case WAIT_OBJECT_0:
- return osl_cond_result_ok;
-
- case WAIT_TIMEOUT:
- return osl_cond_result_timeout;
+ case WAIT_TIMEOUT:
+ return osl_cond_result_timeout;
- default:
- return osl_cond_result_error;
- }
+ default:
+ return osl_cond_result_error;
}
}
diff --git a/vcl/win/dtrans/MtaOleClipb.cxx b/vcl/win/dtrans/MtaOleClipb.cxx
index 494c89adc822..994587b21f64 100644
--- a/vcl/win/dtrans/MtaOleClipb.cxx
+++ b/vcl/win/dtrans/MtaOleClipb.cxx
@@ -45,6 +45,7 @@
#include <systools/win32/comtools.hxx>
#include <systools/win32/retry_if_failed.hxx>
+#include <systools/win32/wait_for_multiple_objects.hxx>
#include <comphelper/windowserrorstring.hxx>
@@ -67,8 +68,6 @@ namespace /* private */
const bool INIT_NONSIGNALED = false;
/* Similar to osl conditions, with two condition objects passed to the
Wait function.
- COM Proxy-Stub communication uses SendMessages for
- synchronization purposes.
*/
class Win32Condition
{
@@ -81,37 +80,15 @@ namespace /* private */
// leave messages sent through
bool wait(HANDLE hEvtAbort)
{
- HANDLE hWaitArray[2] = { m_hEvent, hEvtAbort };
- while (true)
+ const HANDLE hWaitArray[2] = { m_hEvent, hEvtAbort };
+ switch (sal::systools::WaitForMultipleObjects_COMDispatch(2,
hWaitArray, INFINITE))
{
- DWORD dwResult;
- HRESULT hr = CoWaitForMultipleHandles(COWAIT_DISPATCH_CALLS,
INFINITE, 2,
- hWaitArray, &dwResult);
- if (FAILED(hr))
- return false;
-
- switch (dwResult)
- {
case WAIT_OBJECT_0: // wait successful
return true;
case WAIT_OBJECT_0 + 1: // wait aborted
- return false;
-
- case WAIT_OBJECT_0 + 2:
- {
- /* PeekMessage processes all messages in the
SendMessage
- queue that's what we want, messages from the
PostMessage
- queue stay untouched */
- MSG msg;
- PeekMessageW(&msg, nullptr, 0, 0, PM_NOREMOVE);
-
- break;
- }
-
default: // WAIT_FAILED?
return false;
- }
}
}
commit 9d8b4403e640f26df308a57739fee378f5a86121
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Fri Jan 10 11:52:51 2025 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Mon Feb 17 16:34:12 2025 +0500
Try to use CoWaitForMultipleHandles magic to handle COM message loop
A customer reports a freeze problem, with the call stacks seemengly
indicating
a shuck COM re-entry, even though osl_waitCondition should be prepared for
COM
re-entry. The two relevant threads are:
1. VCL Main
win32u.dll!NtUserMsgWaitForMultipleObjectsEx()
user32.dll!RealMsgWaitForMultipleObjectsEx()
combase.dll!CCliModalLoop::BlockFn(void * * ahEvent, unsigned long
cEvents, unsigned long * lpdwSignaled) Line 2156
[Inlineframe] combase.dll!ModalLoop(CSyncClientCall *) Line 164
combase.dll!ClassicSTAThreadWaitForCall(CSyncClientCall * pClientCall,
WaitForCallReason reason, unsigned long dwRetryTimeout) Line 172
[Inlineframe] combase.dll!ThreadSendReceive(tagRPCOLEMESSAGE *) Line 7355
[Inlineframe]
combase.dll!CSyncClientCall::SwitchAptAndDispatchCall(tagRPCOLEMESSAGE *
pMessage) Line 5900
combase.dll!CSyncClientCall::SendReceive2(tagRPCOLEMESSAGE * pMessage,
unsigned long * pstatus) Line 5459
[Inlineframe]
combase.dll!SyncClientCallRetryContext::SendReceiveWithRetry(tagRPCOLEMESSAGE
*) Line 1542
[Inlineframe]
combase.dll!CSyncClientCall::SendReceiveInRetryContext(SyncClientCallRetryContext
*) Line 565
combase.dll!ClassicSTAThreadSendReceive(CSyncClientCall * pClientCall,
tagRPCOLEMESSAGE * pMsg, unsigned long * pulStatus) Line 547
combase.dll!CSyncClientCall::SendReceive(tagRPCOLEMESSAGE * pMessage,
unsigned long * pulStatus) Line 783
combase.dll!CClientChannel::SendReceive(tagRPCOLEMESSAGE * pMessage,
unsigned long * pulStatus) Line 655
combase.dll!NdrExtpProxySendReceive(void * pThis, _MIDL_STUB_MESSAGE *
pStubMsg) Line 2002
rpcrt4.dll!NdrpClientCall3()
rpcrt4.dll!NdrClientCall3()
[Inlineframe] combase.dll!IDataObject_RemoteGetData_Proxy(IDataObject *)
Line 2082
combase.dll!IDataObject_GetData_Proxy(IDataObject * This, tagFORMATETC *
pformatetcIn, tagSTGMEDIUM * pMedium) Line 1267
ole32.dll!CDefObject::GetData(tagFORMATETC * pformatetcIn, tagSTGMEDIUM *
pmedium) Line 1316
[Inlineframe] emboleobj.dll!rtl::OUString::indexOf(const char[13] &) Line
2030
[Inlineframe] emboleobj.dll!GetAspectFromFlavor(const
com::sun::star::datatransfer::DataFlavor &) Line 215
emboleobj.dll!OleComponent::getTransferData(const
com::sun::star::datatransfer::DataFlavor & aFlavor) Line 1556
[Inlineframe] emboleobj.dll!com::sun::star::uno::Type::{ctor}() Line 46
[Inlineframe]
emboleobj.dll!com::sun::star::datatransfer::DataFlavor::{ctor}() Line 20
[Inlineframe]
emboleobj.dll!com::sun::star::embed::VisualRepresentation::{ctor}() Line 19
emboleobj.dll!OleEmbeddedObject::getPreferredVisualRepresentation(__int64
nAspect) Line 372
comphelper.dll!comphelper::EmbeddedObjectContainer::GetGraphicReplacementStream(__int64
nViewAspect, const
com::sun::star::uno::Reference<com::sun::star::embed::XEmbeddedObject> & xObj,
rtl::OUString * pMediaType) Line 1430
[Inlineframe]
svtlo.dll!svt::EmbeddedObjectRef::GetGraphicReplacementStream(__int64) Line 916
svtlo.dll!svt::EmbeddedObjectRef::GetGraphicStream(bool bUpdate) Line 690
svtlo.dll!svt::EmbeddedObjectRef::GetReplacement(bool bUpdate) Line 486
svtlo.dll!svt::EmbeddedObjectRef::GetGraphic() Line 524
swlo.dll!OutHTML_FrameFormatOLENodeGrf(SwHTMLWriter & rWrt, const
SwFrameFormat & rFrameFormat, bool bInCntnr, bool bWriteReplacementGraphic)
Line 1652
swlo.dll!SwHTMLWriter::OutFrameFormat(AllHtmlFlags nMode, const
SwFrameFormat & rFrameFormat, const SdrObject * pSdrObject) Line 464
swlo.dll!OutHTML_SwFlyCnt(SwHTMLWriter & rWrt, const SfxPoolItem & rHt)
Line 2974
swlo.dll!Out(SwHTMLWriter &(*)(SwHTMLWriter &, const SfxPoolItem &) *
pTab, const SfxPoolItem & rHt, SwHTMLWriter & rWrt) Line 40
swlo.dll!OutHTML_SwTextNode(SwHTMLWriter & rWrt, const SwContentNode &
rNode) Line 2477
swlo.dll!SwHTMLWriter::Out_SwDoc(SwPaM * pPam) Line 960
swlo.dll!SwHTMLWriter::WriteStream() Line 612
swlo.dll!Writer::Write(SwPaM & rPaM, SvStream & rStrm, const
rtl::OUString * pFName) Line 236
swlo.dll!Writer::Write(SwPaM & rPam, SfxMedium & rMedium, const
rtl::OUString * pFileName) Line 250
swlo.dll!SwWriter::Write(const tools::SvRef<Writer> & rxWriter, const
rtl::OUString * pRealFileName) Line 872
swlo.dll!SwDocShell::ConvertTo(SfxMedium & rMedium) Line 778
sfxlo.dll!SfxObjectShell::SaveTo_Impl(SfxMedium & rMedium, const
SfxItemSet * pSet) Line 1633
sfxlo.dll!SfxObjectShell::PreDoSaveAs_Impl(const rtl::OUString &
rFileName, const rtl::OUString & aFilterName, const SfxItemSet & rItemSet,
const com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> &
rArgs) Line 3120
sfxlo.dll!SfxObjectShell::CommonSaveAs_Impl(const INetURLObject & aURL,
const rtl::OUString & aFilterName, SfxItemSet & rItemSet, const
com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> & rArgs)
Line 2910
sfxlo.dll!SfxObjectShell::APISaveAs_Impl(std::basic_string_view<char16_t,std::char_traits<char16_t>>
aFileName, SfxItemSet & rItemSet, const
com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> & rArgs)
Line 320
sfxlo.dll!SfxBaseModel::impl_store(const rtl::OUString & sURL, const
com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> &
seqArguments, bool bSaveTo) Line 3203
[Inlineframe]
sfxlo.dll!SfxBaseModel::storeToURL::__l7::<lambda_1>::operator()() Line 1813
sfxlo.dll!vcl::solarthread::detail::GenericSolarThreadExecutor<`SfxBaseModel::storeToURL'::`7'::<lambda_1>,void>::doIt()
Line 114
[Inlineframe] vcllo.dll!vcl::SolarThreadExecutor::worker(void *) Line 37
vcllo.dll!vcl::SolarThreadExecutor::LinkStubworker(void * instance, void
* data) Line 32
[Inlineframe] vcllo.dll!Link<void *,void>::Call(void *) Line 111
[Inlineframe] vcllo.dll!ImplHandleUserEvent(ImplSVEvent *) Line 2287
vcllo.dll!ImplWindowFrameProc(vcl::Window * _pWindow, SalEvent nEvent,
const void * pEvent) Line 2851
vcllo.dll!SalFrame::CallCallback(SalEvent nEvent, const void * pEvent)
Line 310
[Inlineframe] vclplug_winlo.dll!ImplHandleAppCommand(HWND__ *) Line 5496
vclplug_winlo.dll!SalFrameWndProc(HWND__ * hWnd, unsigned int nMsg,
unsigned __int64 wParam, __int64 lParam, bool & rDef) Line 6159
vclplug_winlo.dll!SalFrameWndProc(HWND__ * hWnd, unsigned int nMsg,
unsigned __int64 wParam, __int64 lParam, bool & rDef) Line 6190
user32.dll!UserCallWinProcCheckWow(struct _ACTIVATION_CONTEXT *,__int64
(*)(struct tagWND *,unsigned int,unsigned __int64,__int64),struct HWND__ *,enum
_WM_VALUE,unsigned __int64,__int64,void *,int)
user32.dll!DispatchMessageWorker()
vclplug_winlo.dll!ImplSalDispatchMessage(const tagMSG * pMsg) Line 476
vclplug_winlo.dll!ImplSalYield(bool bWait, bool bHandleAllCurrentEvents)
Line 544
vclplug_winlo.dll!WinSalInstance::DoYield(bool bWait, bool
bHandleAllCurrentEvents) Line 581
vcllo.dll!ImplYield(bool i_bWait, bool i_bAllEvents) Line 393
vcllo.dll!Application::Execute() Line 369
sofficeapp.dll!desktop::Desktop::Main() Line 1605
vcllo.dll!ImplSVMain() Line 229
sofficeapp.dll!soffice_main() Line 94
vText.dll!00007ff7f1bb105b()
vText.dll!00007ff7f1bb1304()
kernel32.dll!00007fff114b7374()
ntdll.dll!00007fff128fcc91()
2. cppu_threadpool::ORequestThread
win32u.dll!NtUserMsgWaitForMultipleObjectsEx()
user32.dll!RealMsgWaitForMultipleObjectsEx()
sal3.dll!osl_waitCondition(void * Condition, const TimeValue * pTimeout)
Line 82
[Inlineframe] vcllo.dll!osl::Condition::wait(const TimeValue *) Line 123
vcllo.dll!vcl::SolarThreadExecutor::execute() Line 63
sfxlo.dll!vcl::solarthread::detail::GenericSolarThreadExecutor<`SfxBaseModel::storeToURL'::`7'::<lambda_1>,void>::exec(const
SfxBaseModel::storeToURL::__l7::<lambda_1> & func) Line 103
[Inlineframe] sfxlo.dll!vcl::solarthread::syncExecute(const
SfxBaseModel::storeToURL::__l7::<lambda_1> &) Line 167
sfxlo.dll!SfxBaseModel::storeToURL(const rtl::OUString & rURL, const
com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> & rArgs)
Line 1813
mscx_uno.dll!`anonymous
namespace'::cpp_call(bridges::cpp_uno::shared::UnoInterfaceProxy * pThis,
bridges::cpp_uno::shared::VtableSlot aVtableSlot,
_typelib_TypeDescriptionReference * pReturnTypeRef, long nParams,
_typelib_MethodParameter * pParams, void * pUnoReturn, void * * pUnoArgs,
_uno_Any * * ppUnoExc) Line 214
mscx_uno.dll!unoInterfaceProxyDispatch(_uno_Interface * pUnoI, const
_typelib_TypeDescription * pMemberTD, void * pReturn, void * * pArgs, _uno_Any
* * ppException) Line 430
binaryurplo.dll!binaryurp::IncomingRequest::execute_throw(binaryurp::BinaryAny
* returnValue,
std::vector<binaryurp::BinaryAny,std::allocator<binaryurp::BinaryAny>> *
outArguments) Line 239
binaryurplo.dll!binaryurp::IncomingRequest::execute() Line 79
binaryurplo.dll!request(void * pThreadSpecificData) Line 84
cppu3.dll!cppu_threadpool::JobQueue::enter(const void * nDisposeId, bool
bReturnWhenNoJob) Line 101
cppu3.dll!cppu_threadpool::ORequestThread::run() Line 169
cppu3.dll!threadFunc(void * param) Line 190
sal3.dll!oslWorkerWrapperFunction(void * pData) Line 67
ucrtbase.dll!00007fff106f1bb2()
kernel32.dll!00007fff114b7374()
ntdll.dll!00007fff128fcc91()
In fact, this is not even obvious that the problem is really re-entry; it
could
also turn out to be a call to an external COM server. I can't debug the
problem
myself, so this is just a guess.
A nice summary of the problem is in
https://stackoverflow.com/questions/34420920;
my suspicion is that our MsgWaitForMultipleObjects call doesn't include some
message types needed for proper COM message pumping, sufficiently defined as
"a small set of special-cased messages" in COWAIT_FLAGS enumeration docs
[1].
Let me try if CoWaitForMultipleHandles in place of MsgWaitForMultipleObjects
maybe resolves the issue.
[1]
https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/ne-combaseapi-cowait_flags
Change-Id: I4eaac32cf08c7379a3f8b88bcdd851e70e97d757
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/180048
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/sal/osl/w32/conditn.cxx b/sal/osl/w32/conditn.cxx
index 6bc6d9d3ed3a..0ad233ed9bff 100644
--- a/sal/osl/w32/conditn.cxx
+++ b/sal/osl/w32/conditn.cxx
@@ -78,8 +78,15 @@ oslConditionResult SAL_CALL osl_waitCondition(oslCondition
Condition,
while ( true )
{
- /* Only wake up if a SendMessage call to the threads message loop is
detected */
- switch( MsgWaitForMultipleObjects( 1, reinterpret_cast<HANDLE
*>(&Condition), FALSE, timeout, QS_SENDMESSAGE ) )
+ DWORD index;
+ /* Only wake up if a SendMessage / COM call to the threads message
loop is detected */
+ HRESULT hr = CoWaitForMultipleHandles(COWAIT_DISPATCH_CALLS, timeout,
1,
+
reinterpret_cast<HANDLE*>(&Condition), &index);
+ if (hr == RPC_S_CALLPENDING)
+ return osl_cond_result_timeout;
+ if (FAILED(hr))
+ return osl_cond_result_error;
+ switch (index)
{
case WAIT_OBJECT_0 + 1:
{
diff --git a/vcl/win/dtrans/MtaOleClipb.cxx b/vcl/win/dtrans/MtaOleClipb.cxx
index a62f4a1c0c72..494c89adc822 100644
--- a/vcl/win/dtrans/MtaOleClipb.cxx
+++ b/vcl/win/dtrans/MtaOleClipb.cxx
@@ -66,11 +66,7 @@ namespace /* private */
const bool MANUAL_RESET = true;
const bool INIT_NONSIGNALED = false;
- /* Cannot use osl conditions because they are blocking
- without waking up on messages sent by another thread
- this leads to deadlocks because we are blocking the
- communication between inter-thread marshalled COM
- pointers.
+ /* Similar to osl conditions, with two condition objects passed to the
Wait function.
COM Proxy-Stub communication uses SendMessages for
synchronization purposes.
*/
@@ -85,11 +81,14 @@ namespace /* private */
// leave messages sent through
bool wait(HANDLE hEvtAbort)
{
- const HANDLE hWaitArray[2] = { m_hEvent, hEvtAbort };
+ HANDLE hWaitArray[2] = { m_hEvent, hEvtAbort };
while (true)
{
- DWORD dwResult
- = MsgWaitForMultipleObjects(2, hWaitArray, FALSE,
INFINITE, QS_SENDMESSAGE);
+ DWORD dwResult;
+ HRESULT hr = CoWaitForMultipleHandles(COWAIT_DISPATCH_CALLS,
INFINITE, 2,
+ hWaitArray, &dwResult);
+ if (FAILED(hr))
+ return false;
switch (dwResult)
{
