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 <sbergman@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 <xiscofauli@libreoffice.org>

4 files changed, 157 insertions, 49 deletions

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,