vcl/win/dtrans/MtaOleClipb.cxx | 137 +++++++++++++++++++++++++++++++++++-----
vcl/win/dtrans/MtaOleClipb.hxx | 26 ++++++-
vcl/win/dtrans/WinClipboard.cxx | 38 ++---------
vcl/win/dtrans/WinClipboard.hxx | 5 -
4 files changed, 157 insertions(+), 49 deletions(-)
New commits:
commit f9c3a734221228cdf5a52ed6ebf9c0a3c1d44607
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Tue Jan 12 09:12:32 2021 +0100
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Wed Jan 13 20:15:21 2021 +0100
tdf#139074: Revert "WIN replace clipboard update thread with Idle" et al
This reverts commit 9617bc9544cd569066ff187c3672a87fe28fe17f "WIN replace
clipboard update thread with Idle" plus follow-up commits
f5ab8bcbfd20ecce4a358f62ee3f81b8b968a5de "WIN don't notify clipboard change
with
SolarMutex" and c921f9bd64187823af2356d7a8ceb77444c17219 "Release solar
mutex
before using an apartment-threaded COM object".
The Gerrit Jenkins Windows builds had started to abort after timeout for
almost
all builds now. Going back to before the youngest of the above three
commits,
c921f9bd64187823af2356d7a8ceb77444c17219 "Release solar mutex before using
an
apartment-threaded COM object" did not improve things (see the
<https://gerrit.libreoffice.org/c/core/+/109100> "Test build #1, DO NOT
SUBMIT"
chain, where three out of three of the Gerrit Jenkins Windows builds timed
out).
But going back to before the oldest of the above three commits,
9617bc9544cd569066ff187c3672a87fe28fe17f "WIN replace clipboard update
thread
with Idle", does look promising (see the
<https://gerrit.libreoffice.org/c/core/+/109155> "Test build #7, DO NOT
SUBMIT"
chain, where three out of three of the Gerrit Jenkins Windows builds
succeeded).
So the hope is that reverting all three commits brings back a green master,
allowing us to understand and fix the actual issue later.
Change-Id: Ie83ba742f216396b49f561d19c2cda7758740502
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/109158
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sberg...@redhat.com>
(cherry picked from commit 694b400f56842cd29ad1a960853cde5aef91e4f0)
Additional information regarding the backport to libreoffice-7-1:
c921f9bd64187823af2356d7a8ceb77444c17219 "Release solar mutex
before using an apartment-threaded COM object" was never backported to
libreoffice-7-1 so this patch only reverts
9617bc9544cd569066ff187c3672a87fe28fe17f "WIN replace
clipboard update thread with Idle" and
f5ab8bcbfd20ecce4a358f62ee3f81b8b968a5de "WIN don't notify clipboard change
with
SolarMutex" in libreoffice-7-1 branch.
The backport was done using gerrit's interface by Xisco Fauli and no
conflict was found while cherry-picking it.
However, this change wasn't backported:
--- a/vcl/win/dtrans/WinClipboard.cxx
+++ b/vcl/win/dtrans/WinClipboard.cxx
@@ -91,9 +84,6 @@ CWinClipboard::~CWinClipboard()
uno::Reference<datatransfer::XTransferable> SAL_CALL
CWinClipboard::getContents()
{
- DBG_TESTSOLARMUTEX();
- SolarMutexReleaser aReleaser;
-
osl::MutexGuard aGuard(m_aMutex);
so it was added manually.
See https://gerrit.libreoffice.org/c/core/+/109115
Change-Id: Ie83ba742f216396b49f561d19c2cda7758740502
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/109115
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
diff --git a/vcl/win/dtrans/MtaOleClipb.cxx b/vcl/win/dtrans/MtaOleClipb.cxx
index 7b8b2b637385..d894ae7b5a48 100644
--- a/vcl/win/dtrans/MtaOleClipb.cxx
+++ b/vcl/win/dtrans/MtaOleClipb.cxx
@@ -45,7 +45,12 @@
#include <systools/win32/comtools.hxx>
-namespace
+// namespace directives
+
+using osl::MutexGuard;
+using osl::ClearableMutexGuard;
+
+namespace /* private */
{
const wchar_t g_szWndClsName[] = L"MtaOleReqWnd###";
@@ -226,7 +231,12 @@ CMtaOleClipboard::CMtaOleClipboard( ) :
m_hwndMtaOleReqWnd( nullptr ),
// signals that the window is destroyed - to stop waiting any winproc
result
m_hEvtWndDisposed(CreateEventW(nullptr, MANUAL_RESET, INIT_NONSIGNALED,
nullptr)),
- m_pfncClipViewerCallback(nullptr)
+ m_MtaOleReqWndClassAtom( 0 ),
+ m_pfncClipViewerCallback( nullptr ),
+ m_bRunClipboardNotifierThread( true ),
+ m_hClipboardChangedEvent( m_hClipboardChangedNotifierEvents[0] ),
+ m_hTerminateClipboardChangedNotifierEvent(
m_hClipboardChangedNotifierEvents[1] ),
+ m_ClipboardChangedEventCount( 0 )
{
OSL_ASSERT( nullptr != m_hEvtThrdReady );
SAL_WARN_IF(!m_hEvtWndDisposed, "dtrans", "CreateEventW failed:
m_hEvtWndDisposed is nullptr");
@@ -236,6 +246,20 @@ CMtaOleClipboard::CMtaOleClipboard( ) :
m_hOleThread = reinterpret_cast<HANDLE>(_beginthreadex(
nullptr, 0, CMtaOleClipboard::oleThreadProc, this, 0, &m_uOleThreadId
));
OSL_ASSERT( nullptr != m_hOleThread );
+
+ // setup the clipboard changed notifier thread
+
+ m_hClipboardChangedNotifierEvents[0] = CreateEventW( nullptr,
MANUAL_RESET, INIT_NONSIGNALED, nullptr );
+ OSL_ASSERT( nullptr != m_hClipboardChangedNotifierEvents[0] );
+
+ m_hClipboardChangedNotifierEvents[1] = CreateEventW( nullptr,
MANUAL_RESET, INIT_NONSIGNALED, nullptr );
+ OSL_ASSERT( nullptr != m_hClipboardChangedNotifierEvents[1] );
+
+ unsigned uThreadId;
+ m_hClipboardChangedNotifierThread =
reinterpret_cast<HANDLE>(_beginthreadex(
+ nullptr, 0, CMtaOleClipboard::clipboardChangedNotifierThreadProc,
this, 0, &uThreadId ));
+
+ OSL_ASSERT( nullptr != m_hClipboardChangedNotifierThread );
}
// dtor
@@ -246,13 +270,35 @@ CMtaOleClipboard::~CMtaOleClipboard( )
if ( nullptr != m_hEvtThrdReady )
ResetEvent( m_hEvtThrdReady );
+ // terminate the clipboard changed notifier thread
+ m_bRunClipboardNotifierThread = false;
+ SetEvent( m_hTerminateClipboardChangedNotifierEvent );
+
+ // unblock whoever could still wait for event processing
+ if (m_hEvtWndDisposed)
+ SetEvent(m_hEvtWndDisposed);
+
+ sal_uInt32 dwResult = WaitForSingleObject(
+ m_hClipboardChangedNotifierThread, MAX_WAIT_SHUTDOWN );
+
+ OSL_ENSURE( dwResult == WAIT_OBJECT_0, "clipboard notifier thread could
not terminate" );
+
+ if ( nullptr != m_hClipboardChangedNotifierThread )
+ CloseHandle( m_hClipboardChangedNotifierThread );
+
+ if ( nullptr != m_hClipboardChangedNotifierEvents[0] )
+ CloseHandle( m_hClipboardChangedNotifierEvents[0] );
+
+ if ( nullptr != m_hClipboardChangedNotifierEvents[1] )
+ CloseHandle( m_hClipboardChangedNotifierEvents[1] );
+
// end the thread
// because DestroyWindow can only be called
// from within the thread that created the window
sendMessage( MSG_SHUTDOWN );
// wait for thread shutdown
- DWORD dwResult = WaitForSingleObject(m_hOleThread, MAX_WAIT_SHUTDOWN);
+ dwResult = WaitForSingleObject( m_hOleThread, MAX_WAIT_SHUTDOWN );
OSL_ENSURE( dwResult == WAIT_OBJECT_0, "OleThread could not terminate" );
if ( nullptr != m_hOleThread )
@@ -264,6 +310,9 @@ CMtaOleClipboard::~CMtaOleClipboard( )
if (m_hEvtWndDisposed)
CloseHandle(m_hEvtWndDisposed);
+ if ( m_MtaOleReqWndClassAtom )
+ UnregisterClassW( g_szWndClsName, nullptr );
+
OSL_ENSURE( ( nullptr == m_pfncClipViewerCallback ),
"Clipboard viewer not properly unregistered" );
}
@@ -299,6 +348,7 @@ HRESULT CMtaOleClipboard::getClipboard( IDataObject**
ppIDataObject )
}
CAutoComInit comAutoInit;
+
LPSTREAM lpStream;
*ppIDataObject = nullptr;
@@ -383,6 +433,10 @@ bool CMtaOleClipboard::onRegisterClipViewer(
LPFNC_CLIPVIEWER_CALLBACK_t pfncCli
{
bool bRet = false;
+ // we need exclusive access because the clipboard changed notifier
+ // thread also accesses this variable
+ MutexGuard aGuard( m_pfncClipViewerCallbackMutex );
+
// register if not yet done
if ( ( nullptr != pfncClipViewerCallback ) && ( nullptr ==
m_pfncClipViewerCallback ) )
{
@@ -441,8 +495,14 @@ LRESULT CMtaOleClipboard::onClipboardUpdate()
{
// we don't send a notification if we are
// registering ourself as clipboard
- if (!m_bInRegisterClipViewer && m_pfncClipViewerCallback)
- m_pfncClipViewerCallback();
+ if ( !m_bInRegisterClipViewer )
+ {
+ MutexGuard aGuard( m_ClipboardChangedEventCountMutex );
+
+ m_ClipboardChangedEventCount++;
+ SetEvent( m_hClipboardChangedEvent );
+ }
+
return 0;
}
@@ -546,11 +606,8 @@ LRESULT CALLBACK CMtaOleClipboard::mtaOleReqWndProc( HWND
hWnd, UINT uMsg, WPARA
return lResult;
}
-unsigned int CMtaOleClipboard::run()
+void CMtaOleClipboard::createMtaOleReqWnd( )
{
- HRESULT hr = OleInitialize(nullptr);
- OSL_ASSERT(SUCCEEDED(hr));
-
WNDCLASSEXW wcex;
SalData* pSalData = GetSalData();
@@ -571,11 +628,19 @@ unsigned int CMtaOleClipboard::run()
wcex.lpszClassName = g_szWndClsName;
wcex.hIconSm = nullptr;
- ATOM aMtaOleReqWndClassAtom = RegisterClassExW(&wcex);
+ m_MtaOleReqWndClassAtom = RegisterClassExW( &wcex );
- if (0 != aMtaOleReqWndClassAtom)
+ if ( 0 != m_MtaOleReqWndClassAtom )
m_hwndMtaOleReqWnd = CreateWindowW(
g_szWndClsName, nullptr, 0, 0, 0, 0, 0, nullptr, nullptr,
pSalData->mhInst, nullptr );
+}
+
+unsigned int CMtaOleClipboard::run( )
+{
+ HRESULT hr = OleInitialize( nullptr );
+ OSL_ASSERT( SUCCEEDED( hr ) );
+
+ createMtaOleReqWnd( );
unsigned int nRet;
@@ -594,9 +659,6 @@ unsigned int CMtaOleClipboard::run()
else
nRet = ~0U;
- if (aMtaOleReqWndClassAtom)
- UnregisterClassW(g_szWndClsName, nullptr);
-
OleUninitialize( );
return nRet;
@@ -613,6 +675,53 @@ unsigned int WINAPI CMtaOleClipboard::oleThreadProc(
LPVOID pParam )
return pInst->run( );
}
+unsigned int WINAPI CMtaOleClipboard::clipboardChangedNotifierThreadProc(
LPVOID pParam )
+{
+
osl_setThreadName("CMtaOleClipboard::clipboardChangedNotifierThreadProc()");
+ CMtaOleClipboard* pInst = static_cast< CMtaOleClipboard* >( pParam );
+ OSL_ASSERT( nullptr != pInst );
+
+ CoInitializeEx( nullptr, COINIT_APARTMENTTHREADED );
+
+ // assuming we don't need a lock for
+ // a boolean variable like m_bRun...
+ while ( pInst->m_bRunClipboardNotifierThread )
+ {
+ // process window messages because of CoInitializeEx
+ MSG Msg;
+ while (PeekMessageW(&Msg, nullptr, 0, 0, PM_REMOVE))
+ DispatchMessageW(&Msg);
+
+ // wait for clipboard changed or terminate event
+ MsgWaitForMultipleObjects(2, pInst->m_hClipboardChangedNotifierEvents,
false, INFINITE,
+ QS_ALLINPUT | QS_ALLPOSTMESSAGE);
+
+ ClearableMutexGuard aGuard( pInst->m_ClipboardChangedEventCountMutex );
+
+ if ( pInst->m_ClipboardChangedEventCount > 0 )
+ {
+ pInst->m_ClipboardChangedEventCount--;
+ if ( 0 == pInst->m_ClipboardChangedEventCount )
+ ResetEvent( pInst->m_hClipboardChangedEvent );
+
+ aGuard.clear( );
+
+ // nobody should touch m_pfncClipViewerCallback while we do
+ MutexGuard aClipViewerGuard( pInst->m_pfncClipViewerCallbackMutex
);
+
+ // notify all clipboard listener
+ if ( pInst->m_pfncClipViewerCallback )
+ pInst->m_pfncClipViewerCallback( );
+ }
+ else
+ aGuard.clear( );
+ }
+
+ CoUninitialize( );
+
+ return 0;
+}
+
bool CMtaOleClipboard::WaitForThreadReady( ) const
{
bool bRet = false;
diff --git a/vcl/win/dtrans/MtaOleClipb.hxx b/vcl/win/dtrans/MtaOleClipb.hxx
index d0dd539d46a5..c406f81aafd3 100644
--- a/vcl/win/dtrans/MtaOleClipb.hxx
+++ b/vcl/win/dtrans/MtaOleClipb.hxx
@@ -20,6 +20,7 @@
#pragma once
#include <sal/types.h>
+#include <osl/mutex.hxx>
#include <objidl.h>
@@ -30,14 +31,12 @@
// only from within the clipboard service and the methods
// of the clipboard service are already synchronized
-// Its thread creates a hidden window, which serves as a request target, so we
-// can guarantee synchronization.
-
class CMtaOleClipboard
{
public:
typedef void ( WINAPI *LPFNC_CLIPVIEWER_CALLBACK_t )( void );
+public:
CMtaOleClipboard( );
~CMtaOleClipboard( );
@@ -56,12 +55,17 @@ public:
private:
unsigned int run( );
+ // create a hidden window which serves as a request target; so we
+ // guarantee synchronization
+ void createMtaOleReqWnd( );
+
// message support
bool postMessage( UINT msg, WPARAM wParam = 0, LPARAM lParam = 0 );
LRESULT sendMessage( UINT msg, WPARAM wParam = 0, LPARAM lParam = 0 );
// message handler functions; remember these functions are called
// from a different thread context!
+
static HRESULT onSetClipboard( IDataObject* pIDataObject );
static HRESULT onGetClipboard( LPSTREAM* ppStream );
static HRESULT onFlushClipboard( );
@@ -73,18 +77,34 @@ private:
static LRESULT CALLBACK mtaOleReqWndProc( HWND hWnd, UINT uMsg, WPARAM
wParam, LPARAM lParam );
static unsigned int WINAPI oleThreadProc( LPVOID pParam );
+ static unsigned int WINAPI clipboardChangedNotifierThreadProc( LPVOID
pParam );
+
bool WaitForThreadReady( ) const;
+private:
HANDLE m_hOleThread;
unsigned m_uOleThreadId;
HANDLE m_hEvtThrdReady;
HWND m_hwndMtaOleReqWnd;
HANDLE m_hEvtWndDisposed;
+ ATOM m_MtaOleReqWndClassAtom;
LPFNC_CLIPVIEWER_CALLBACK_t m_pfncClipViewerCallback;
bool m_bInRegisterClipViewer;
+ bool m_bRunClipboardNotifierThread;
+ HANDLE m_hClipboardChangedNotifierThread;
+ HANDLE m_hClipboardChangedNotifierEvents[2];
+ HANDLE& m_hClipboardChangedEvent;
+ HANDLE& m_hTerminateClipboardChangedNotifierEvent;
+ osl::Mutex m_ClipboardChangedEventCountMutex;
+ sal_Int32 m_ClipboardChangedEventCount;
+
+ osl::Mutex m_pfncClipViewerCallbackMutex;
+
static CMtaOleClipboard* s_theMtaOleClipboardInst;
+// not allowed
+private:
CMtaOleClipboard( const CMtaOleClipboard& );
CMtaOleClipboard& operator=( const CMtaOleClipboard& );
diff --git a/vcl/win/dtrans/WinClipboard.cxx b/vcl/win/dtrans/WinClipboard.cxx
index 6d1b03b0887e..f553ba3dd34e 100644
--- a/vcl/win/dtrans/WinClipboard.cxx
+++ b/vcl/win/dtrans/WinClipboard.cxx
@@ -26,8 +26,6 @@
#include <com/sun/star/uno/XComponentContext.hpp>
#include <cppuhelper/supportsservice.hxx>
#include <cppuhelper/weak.hxx>
-#include <tools/debug.hxx>
-#include <vcl/svapp.hxx>
#include <com/sun/star/datatransfer/clipboard/RenderingCapabilities.hpp>
#include "XNotifyingDataObject.hxx"
@@ -64,11 +62,6 @@ CWinClipboard::CWinClipboard(const
uno::Reference<uno::XComponentContext>& rxCon
// the static callback function
s_pCWinClipbImpl = this;
registerClipboardViewer();
-
-
m_aNotifyClipboardChangeIdle.SetDebugName("CWinClipboard::m_aNotifyClipboardChangeIdle");
- m_aNotifyClipboardChangeIdle.SetPriority(TaskPriority::HIGH_IDLE);
- m_aNotifyClipboardChangeIdle.SetInvokeHandler(
- LINK(this, CWinClipboard, ClipboardContentChangedHdl));
}
CWinClipboard::~CWinClipboard()
@@ -91,9 +84,6 @@ CWinClipboard::~CWinClipboard()
uno::Reference<datatransfer::XTransferable> SAL_CALL
CWinClipboard::getContents()
{
- DBG_TESTSOLARMUTEX();
- SolarMutexReleaser aReleaser;
-
osl::MutexGuard aGuard(m_aMutex);
if (rBHelper.bDisposed)
@@ -245,11 +235,8 @@ void SAL_CALL CWinClipboard::removeClipboardListener(
rBHelper.aLC.removeInterface(cppu::UnoType<decltype(listener)>::get(),
listener);
}
-IMPL_LINK_NOARG(CWinClipboard, ClipboardContentChangedHdl, Timer*, void)
+void CWinClipboard::notifyAllClipboardListener()
{
- DBG_TESTSOLARMUTEX();
- SolarMutexReleaser aReleaser;
-
if (rBHelper.bDisposed)
return;
@@ -266,11 +253,7 @@ IMPL_LINK_NOARG(CWinClipboard, ClipboardContentChangedHdl,
Timer*, void)
try
{
cppu::OInterfaceIteratorHelper iter(*pICHelper);
- uno::Reference<datatransfer::XTransferable> rXTransf;
- {
- SolarMutexGuard aGuard;
- rXTransf.set(getContents());
- }
+ uno::Reference<datatransfer::XTransferable> rXTransf(getContents());
datatransfer::clipboard::ClipboardEvent
aClipbEvent(static_cast<XClipboard*>(this),
rXTransf);
@@ -339,22 +322,21 @@ void
CWinClipboard::onReleaseDataObject(CXNotifyingDataObject* theCaller)
void CWinClipboard::registerClipboardViewer()
{
- m_MtaOleClipboard.registerClipViewer(CWinClipboard::onWM_CLIPBOARDUPDATE);
+
m_MtaOleClipboard.registerClipViewer(CWinClipboard::onClipboardContentChanged);
}
void CWinClipboard::unregisterClipboardViewer() {
m_MtaOleClipboard.registerClipViewer(nullptr); }
-void WINAPI CWinClipboard::onWM_CLIPBOARDUPDATE()
+void WINAPI CWinClipboard::onClipboardContentChanged()
{
osl::MutexGuard aGuard(s_aMutex);
- if (!s_pCWinClipbImpl)
- return;
-
- s_pCWinClipbImpl->m_foreignContent.clear();
-
- if (!s_pCWinClipbImpl->m_aNotifyClipboardChangeIdle.IsActive())
- s_pCWinClipbImpl->m_aNotifyClipboardChangeIdle.Start();
+ // reassociation to instance through static member
+ if (nullptr != s_pCWinClipbImpl)
+ {
+ s_pCWinClipbImpl->m_foreignContent.clear();
+ s_pCWinClipbImpl->notifyAllClipboardListener();
+ }
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/win/dtrans/WinClipboard.hxx b/vcl/win/dtrans/WinClipboard.hxx
index 06bcdce0fc64..1b0a05a3450d 100644
--- a/vcl/win/dtrans/WinClipboard.hxx
+++ b/vcl/win/dtrans/WinClipboard.hxx
@@ -32,7 +32,6 @@
#include <com/sun/star/lang/XServiceInfo.hpp>
#include <com/sun/star/uno/XComponentContext.hpp>
#include <osl/conditn.hxx>
-#include <vcl/Idle.hxx>
#include "MtaOleClipb.hxx"
@@ -71,7 +70,6 @@ class CWinClipboard final
CXNotifyingDataObject* m_pCurrentClipContent;
com::sun::star::uno::Reference<com::sun::star::datatransfer::XTransferable>
m_foreignContent;
osl::Mutex m_ClipContentMutex;
- Idle m_aNotifyClipboardChangeIdle;
static osl::Mutex s_aMutex;
static CWinClipboard* s_pCWinClipbImpl;
@@ -82,8 +80,7 @@ class CWinClipboard final
void registerClipboardViewer();
void unregisterClipboardViewer();
- static void WINAPI onWM_CLIPBOARDUPDATE();
- DECL_DLLPRIVATE_LINK(ClipboardContentChangedHdl, Timer*, void);
+ static void WINAPI onClipboardContentChanged();
public:
CWinClipboard(const css::uno::Reference<css::uno::XComponentContext>&
rxContext,
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
