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 <sbergman@redhat.com>

5 files changed, 159 insertions, 74 deletions

diff --git a/include/vcl/svapp.hxx b/include/vcl/svapp.hxx
index fa5702717086..bfa92cf88321 100644
--- a/include/vcl/svapp.hxx
+++ b/include/vcl/svapp.hxx
@@ -1421,26 +1421,6 @@ public:
     ~SolarMutexReleaser() { Application::AcquireSolarMutex( mnReleased ); }
 };
 
-/**
- A helper class that calls Application::ReleaseSolarMutex() in its constructor
- *if it was acquired* and restores the mutex in its destructor.
-*/
-class SolarMutexReleaserIfAcquired
-{
-    const sal_uInt32 mnReleased;
-public:
-    SolarMutexReleaserIfAcquired()
-        : mnReleased(
-            Application::GetSolarMutex().IsCurrentThread() ? Application::ReleaseSolarMutex() : 0)
-    {
-    }
-    ~SolarMutexReleaserIfAcquired()
-    {
-        if (mnReleased)
-            Application::AcquireSolarMutex(mnReleased);
-    }
-};
-
 VCL_DLLPUBLIC Application* GetpApp();
 
 // returns true if vcl is already initialized
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 1f49bde99192..f50c1810f4ea 100644
--- a/vcl/win/dtrans/WinClipboard.cxx
+++ b/vcl/win/dtrans/WinClipboard.cxx
@@ -27,8 +27,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"
@@ -65,11 +63,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()
@@ -126,14 +119,8 @@ uno::Reference<datatransfer::XTransferable> SAL_CALL CWinClipboard::getContents(
         // com smart pointer to the IDataObject from clipboard
         IDataObjectPtr pIDo(new CAPNDataObject(pIDataObject));
 
-        {
-            // before calling COM methods of an apartment-threaded COM object, release solar mutex
-            // to avoid deadlocking on waiting clipboard thread that would try to acquire it
-            SolarMutexReleaserIfAcquired aReleaser;
-
-            // remember pIDo destroys itself due to the smart pointer
-            rClipContent = CDOTransferable::create(m_xContext, pIDo);
-        }
+        // remember pIDo destroys itself due to the smart pointer
+        rClipContent = CDOTransferable::create(m_xContext, pIDo);
 
         osl::MutexGuard aGuard2(m_ClipContentMutex);
         m_foreignContent = rClipContent;
@@ -249,11 +236,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;
 
@@ -270,11 +254,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);
 
@@ -343,22 +323,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,