Kaydet (Commit) 13a34f4c authored tarafından Jan-Marek Glogowski's avatar Jan-Marek Glogowski Kaydeden (comit) Thorsten Behrens

Rewrite Qt4 based nested yield mutex locking.

The Qt event loop may start a nested event loop, when checking for
clipboard and Drag'n'Drop events.

Previously this was handled by running this nested yield loop
inside the main glib loop using
  qApp->clipboard()->property( "useEventLoopWhenWaiting" );

But this results in nested paint events which crash LO:
  QWidget::repaint: Recursive repaint detected

To prevend yield mutex deadlocks, check for nested event loops
and always release the yield lock before starting the nested Yield
event loop.

This fixes fdo#69002.

Change-Id: I7e827abd3489783053ec7123372742a32555875d
Reviewed-on: https://gerrit.libreoffice.org/6685Reviewed-by: 's avatarMichael Meeks <michael.meeks@collabora.com>
Reviewed-by: 's avatarThorsten Behrens <thb@documentfoundation.org>
Tested-by: 's avatarThorsten Behrens <thb@documentfoundation.org>
üst fa8b5ae8
...@@ -58,6 +58,8 @@ ...@@ -58,6 +58,8 @@
#undef Region #undef Region
#include "generic/geninst.h"
using namespace ::com::sun::star; using namespace ::com::sun::star;
using namespace ::com::sun::star::ui::dialogs; using namespace ::com::sun::star::ui::dialogs;
using namespace ::com::sun::star::ui::dialogs::TemplateDescription; using namespace ::com::sun::star::ui::dialogs::TemplateDescription;
...@@ -253,28 +255,16 @@ sal_Int16 SAL_CALL KDE4FilePicker::execute() ...@@ -253,28 +255,16 @@ sal_Int16 SAL_CALL KDE4FilePicker::execute()
_dialog->setFilter(_filter); _dialog->setFilter(_filter);
_dialog->filterWidget()->setEditable(false); _dialog->filterWidget()->setEditable(false);
// At this point, SolarMutex is held. Opening the KDE file dialog here // We're entering a nested loop.
// can lead to QClipboard asking for clipboard contents. If LO core // Release the yield mutex to prevent deadlocks.
// is the owner of the clipboard content, this will block for 5 seconds
// and timeout, since the clipboard thread will not be able to acquire
// SolarMutex and thus won't be able to respond. If the event loops
// are properly integrated and QClipboard can use a nested event loop
// (see the KDE VCL plug), then this won't happen, but otherwise
// simply release the SolarMutex here. The KDE file dialog does not
// call back to the core, so this should be safe (and if it does,
// SolarMutex will need to be re-acquired).
long mutexrelease = 0;
if( !qApp->clipboard()->property( "useEventLoopWhenWaiting" ).toBool())
mutexrelease = Application::ReleaseSolarMutex();
//block and wait for user input
int result = _dialog->exec(); int result = _dialog->exec();
// HACK: KFileDialog uses KConfig("kdeglobals") for saving some settings // HACK: KFileDialog uses KConfig("kdeglobals") for saving some settings
// (such as the auto-extension flag), but that doesn't update KGlobal::config() // (such as the auto-extension flag), but that doesn't update KGlobal::config()
// (which is probably a KDE bug), so force reading the new configuration, // (which is probably a KDE bug), so force reading the new configuration,
// otherwise the next opening of the dialog would use the old settings. // otherwise the next opening of the dialog would use the old settings.
KGlobal::config()->reparseConfiguration(); KGlobal::config()->reparseConfiguration();
if( !qApp->clipboard()->property( "useEventLoopWhenWaiting" ).toBool())
Application::AcquireSolarMutex( mutexrelease );
if( result == KFileDialog::Accepted) if( result == KFileDialog::Accepted)
return ExecutableDialogResults::OK; return ExecutableDialogResults::OK;
......
...@@ -225,9 +225,6 @@ void KDEXLib::setupEventLoop() ...@@ -225,9 +225,6 @@ void KDEXLib::setupEventLoop()
eventLoopType = GlibEventLoop; eventLoopType = GlibEventLoop;
old_gpoll = g_main_context_get_poll_func( NULL ); old_gpoll = g_main_context_get_poll_func( NULL );
g_main_context_set_poll_func( NULL, gpoll_wrapper ); g_main_context_set_poll_func( NULL, gpoll_wrapper );
// set QClipboard to use event loop, otherwise the main thread will hold
// SolarMutex locked, which will prevent the clipboard thread from answering
m_pApplication->clipboard()->setProperty( "useEventLoopWhenWaiting", true );
return; return;
} }
#endif #endif
...@@ -244,9 +241,6 @@ void KDEXLib::setupEventLoop() ...@@ -244,9 +241,6 @@ void KDEXLib::setupEventLoop()
eventLoopType = QtUnixEventLoop; eventLoopType = QtUnixEventLoop;
QInternal::callFunction( QInternal::GetUnixSelectFunction, reinterpret_cast< void** >( &qt_select )); QInternal::callFunction( QInternal::GetUnixSelectFunction, reinterpret_cast< void** >( &qt_select ));
QInternal::callFunction( QInternal::SetUnixSelectFunction, reinterpret_cast< void** >( lo_select )); QInternal::callFunction( QInternal::SetUnixSelectFunction, reinterpret_cast< void** >( lo_select ));
// set QClipboard to use event loop, otherwise the main thread will hold
// SolarMutex locked, which will prevent the clipboard thread from answering
m_pApplication->clipboard()->setProperty( "useEventLoopWhenWaiting", true );
return; return;
} }
#endif #endif
...@@ -300,6 +294,9 @@ void KDEXLib::socketNotifierActivated( int fd ) ...@@ -300,6 +294,9 @@ void KDEXLib::socketNotifierActivated( int fd )
void KDEXLib::Yield( bool bWait, bool bHandleAllCurrentEvents ) void KDEXLib::Yield( bool bWait, bool bHandleAllCurrentEvents )
{ {
// Nested yield loop counter.
static int loop_depth = 0;
if( eventLoopType == LibreOfficeEventLoop ) if( eventLoopType == LibreOfficeEventLoop )
{ {
if( qApp->thread() == QThread::currentThread()) if( qApp->thread() == QThread::currentThread())
...@@ -310,13 +307,24 @@ void KDEXLib::Yield( bool bWait, bool bHandleAllCurrentEvents ) ...@@ -310,13 +307,24 @@ void KDEXLib::Yield( bool bWait, bool bHandleAllCurrentEvents )
} }
return SalXLib::Yield( bWait, bHandleAllCurrentEvents ); return SalXLib::Yield( bWait, bHandleAllCurrentEvents );
} }
// if we are the main thread (which is where the event processing is done), // if we are the main thread (which is where the event processing is done),
// good, just do it // good, just do it
if( qApp->thread() == QThread::currentThread()) if( qApp->thread() == QThread::currentThread()) {
// Release the yield lock before entering a nested loop.
if (loop_depth > 0)
SalYieldMutexReleaser aReleaser;
loop_depth++;
processYield( bWait, bHandleAllCurrentEvents ); processYield( bWait, bHandleAllCurrentEvents );
else loop_depth--;
{ // if this deadlocks, event processing needs to go into a separate thread }
// or some other solution needs to be found else {
// we were called from another thread;
// release the yield lock to prevent deadlock.
SalYieldMutexReleaser aReleaser;
// if this deadlocks, event processing needs to go into a separate
// thread or some other solution needs to be found
Q_EMIT processYieldSignal( bWait, bHandleAllCurrentEvents ); Q_EMIT processYieldSignal( bWait, bHandleAllCurrentEvents );
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment