WIN replace clipboard update thread with Idle

Little "fallout" patch when moving code from /dtrans/** to
/vcl/win/dtrans and merging CWinClipbImpl into CWinClipboard
while I tried to reproduce tdf#62196...

And since we now process the notification in the main thread, we
can get rid of m_pfncClipViewerCallbackMutex, which brings us down
from 6 (!) to 4 mutexes (if Mike counted correct) in the Windows
clipboard code... ok, technically the scheduler / Idle adds its
mutex to this count, but that is not related to the clipboard
handling on Windows ;-)

This also moves the UnregisterClassW into the OLE thread, which
already calls RegisterClassW, to be more consistent.

This now also gets merged, because it seem to fix a deadlocks when
running CppunitTest_sc_macros_test in a loop, which is unclear
where it comes from and I can't reproduce. Tinderboxes and Gerrit
also still seem fine.

Change-Id: Iacbda0bdf6c98f27f6e59964d541524cb45ade24
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107168
Tested-by: Jenkins
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>
(cherry picked from commit 9617bc9544cd569066ff187c3672a87fe28fe17f)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107622
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

4 files changed, 36 insertions, 156 deletions

diff --git a/vcl/win/dtrans/MtaOleClipb.cxx b/vcl/win/dtrans/MtaOleClipb.cxx
index d894ae7b5a48..7b8b2b637385 100644
--- a/vcl/win/dtrans/MtaOleClipb.cxx
+++ b/vcl/win/dtrans/MtaOleClipb.cxx
@@ -45,12 +45,7 @@
 
 #include <systools/win32/comtools.hxx>
 
-//  namespace directives
-
-using osl::MutexGuard;
-using osl::ClearableMutexGuard;
-
-namespace /* private */
+namespace
 {
     const wchar_t g_szWndClsName[] = L"MtaOleReqWnd###";
 
@@ -231,12 +226,7 @@ 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_MtaOleReqWndClassAtom( 0 ),
-    m_pfncClipViewerCallback( nullptr ),
-    m_bRunClipboardNotifierThread( true ),
-    m_hClipboardChangedEvent( m_hClipboardChangedNotifierEvents[0] ),
-    m_hTerminateClipboardChangedNotifierEvent( m_hClipboardChangedNotifierEvents[1] ),
-    m_ClipboardChangedEventCount( 0 )
+    m_pfncClipViewerCallback(nullptr)
 {
     OSL_ASSERT( nullptr != m_hEvtThrdReady );
     SAL_WARN_IF(!m_hEvtWndDisposed, "dtrans", "CreateEventW failed: m_hEvtWndDisposed is nullptr");
@@ -246,20 +236,6 @@ 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
@@ -270,35 +246,13 @@ 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
-    dwResult = WaitForSingleObject( m_hOleThread, MAX_WAIT_SHUTDOWN );
+    DWORD dwResult = WaitForSingleObject(m_hOleThread, MAX_WAIT_SHUTDOWN);
     OSL_ENSURE( dwResult == WAIT_OBJECT_0, "OleThread could not terminate" );
 
     if ( nullptr != m_hOleThread )
@@ -310,9 +264,6 @@ 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" );
 }
@@ -348,7 +299,6 @@ HRESULT CMtaOleClipboard::getClipboard( IDataObject** ppIDataObject )
     }
 
     CAutoComInit comAutoInit;
-
     LPSTREAM lpStream;
 
     *ppIDataObject = nullptr;
@@ -433,10 +383,6 @@ 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 ) )
     {
@@ -495,14 +441,8 @@ LRESULT CMtaOleClipboard::onClipboardUpdate()
 {
     // we don't send a notification if we are
     // registering ourself as clipboard
-    if ( !m_bInRegisterClipViewer )
-    {
-        MutexGuard aGuard( m_ClipboardChangedEventCountMutex );
-
-        m_ClipboardChangedEventCount++;
-        SetEvent( m_hClipboardChangedEvent );
-    }
-
+    if (!m_bInRegisterClipViewer && m_pfncClipViewerCallback)
+        m_pfncClipViewerCallback();
     return 0;
 }
 
@@ -606,8 +546,11 @@ LRESULT CALLBACK CMtaOleClipboard::mtaOleReqWndProc( HWND hWnd, UINT uMsg, WPARA
     return lResult;
 }
 
-void CMtaOleClipboard::createMtaOleReqWnd( )
+unsigned int CMtaOleClipboard::run()
 {
+    HRESULT hr = OleInitialize(nullptr);
+    OSL_ASSERT(SUCCEEDED(hr));
+
     WNDCLASSEXW  wcex;
 
     SalData* pSalData = GetSalData();
@@ -628,19 +571,11 @@ void CMtaOleClipboard::createMtaOleReqWnd( )
     wcex.lpszClassName  = g_szWndClsName;
     wcex.hIconSm        = nullptr;
 
-    m_MtaOleReqWndClassAtom = RegisterClassExW( &wcex );
+    ATOM aMtaOleReqWndClassAtom = RegisterClassExW(&wcex);
 
-    if ( 0 != m_MtaOleReqWndClassAtom )
+    if (0 != aMtaOleReqWndClassAtom)
         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;
 
@@ -659,6 +594,9 @@ unsigned int CMtaOleClipboard::run( )
     else
         nRet = ~0U;
 
+    if (aMtaOleReqWndClassAtom)
+        UnregisterClassW(g_szWndClsName, nullptr);
+
     OleUninitialize( );
 
     return nRet;
@@ -675,53 +613,6 @@ 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 c406f81aafd3..d0dd539d46a5 100644
--- a/vcl/win/dtrans/MtaOleClipb.hxx
+++ b/vcl/win/dtrans/MtaOleClipb.hxx
@@ -20,7 +20,6 @@
 #pragma once
 
 #include <sal/types.h>
-#include <osl/mutex.hxx>
 
 #include <objidl.h>
 
@@ -31,12 +30,14 @@
 // 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( );
 
@@ -55,17 +56,12 @@ 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( );
@@ -77,34 +73,18 @@ 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 f553ba3dd34e..6446a33e4574 100644
--- a/vcl/win/dtrans/WinClipboard.cxx
+++ b/vcl/win/dtrans/WinClipboard.cxx
@@ -62,6 +62,11 @@ 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()
@@ -235,8 +240,10 @@ void SAL_CALL CWinClipboard::removeClipboardListener(
     rBHelper.aLC.removeInterface(cppu::UnoType<decltype(listener)>::get(), listener);
 }
 
-void CWinClipboard::notifyAllClipboardListener()
+IMPL_LINK_NOARG(CWinClipboard, ClipboardContentChangedHdl, Timer*, void)
 {
+    m_foreignContent.clear();
+
     if (rBHelper.bDisposed)
         return;
 
@@ -322,21 +329,20 @@ void CWinClipboard::onReleaseDataObject(CXNotifyingDataObject* theCaller)
 
 void CWinClipboard::registerClipboardViewer()
 {
-    m_MtaOleClipboard.registerClipViewer(CWinClipboard::onClipboardContentChanged);
+    m_MtaOleClipboard.registerClipViewer(CWinClipboard::onWM_CLIPBOARDUPDATE);
 }
 
 void CWinClipboard::unregisterClipboardViewer() { m_MtaOleClipboard.registerClipViewer(nullptr); }
 
-void WINAPI CWinClipboard::onClipboardContentChanged()
+void WINAPI CWinClipboard::onWM_CLIPBOARDUPDATE()
 {
     osl::MutexGuard aGuard(s_aMutex);
 
-    // reassociation to instance through static member
-    if (nullptr != s_pCWinClipbImpl)
-    {
-        s_pCWinClipbImpl->m_foreignContent.clear();
-        s_pCWinClipbImpl->notifyAllClipboardListener();
-    }
+    if (!s_pCWinClipbImpl)
+        return;
+
+    if (!s_pCWinClipbImpl->m_aNotifyClipboardChangeIdle.IsActive())
+        s_pCWinClipbImpl->m_aNotifyClipboardChangeIdle.Start();
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/win/dtrans/WinClipboard.hxx b/vcl/win/dtrans/WinClipboard.hxx
index 1b0a05a3450d..06bcdce0fc64 100644
--- a/vcl/win/dtrans/WinClipboard.hxx
+++ b/vcl/win/dtrans/WinClipboard.hxx
@@ -32,6 +32,7 @@
 #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"
 
@@ -70,6 +71,7 @@ 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;
@@ -80,7 +82,8 @@ class CWinClipboard final
     void registerClipboardViewer();
     void unregisterClipboardViewer();
 
-    static void WINAPI onClipboardContentChanged();
+    static void WINAPI onWM_CLIPBOARDUPDATE();
+    DECL_DLLPRIVATE_LINK(ClipboardContentChangedHdl, Timer*, void);
 
 public:
     CWinClipboard(const css::uno::Reference<css::uno::XComponentContext>& rxContext,