tdf#123502: Make sure to hold the mutex before releasing it

Fixes a regression after commit 5b2f1243bd44b450da26c8344b3ee459f8e88f6c

The destruction of the dialog is initiated by a callback executed from
VCLXButton::ProcessWindowEvent, which notifies listeners, using a call
to VCLXWindow::ImplExecuteAsyncWithoutSolarLock. The stack looks like
this:

ucrtbased.dll!issue_debug_notification(const wchar_t * const message) Line 28
at minkernel\crts\ucrt\src\appcrt\internal\report_runtime_error.cpp(28)
ucrtbased.dll!__acrt_report_runtime_error(const wchar_t * message) Line 154
at minkernel\crts\ucrt\src\appcrt\internal\report_runtime_error.cpp(154)
ucrtbased.dll!abort() Line 51
at minkernel\crts\ucrt\src\appcrt\startup\abort.cpp(51)
comphelper.dll!comphelper::SolarMutex::doRelease(bool bUnlockAll) Line 65
at c:\lo\src\core2\comphelper\source\misc\solarmutex.cxx(65)
vclplug_winlo.dll!SalYieldMutex::doRelease(bool bUnlockAll) Line 169
at c:\lo\src\core2\vcl\win\app\salinst.cxx(169)
comphelper.dll!comphelper::SolarMutex::release(bool bUnlockAll) Line 89
at c:\lo\src\core2\include\comphelper\solarmutex.hxx(89)
vcllo.dll!SalInstance::ReleaseYieldMutexAll() Line 114
at c:\lo\src\core2\vcl\source\app\salvtables.cxx(114)
vcllo.dll!Application::ReleaseSolarMutex() Line 548
at c:\lo\src\core2\vcl\source\app\svapp.cxx(548)
fps.dll!SolarMutexReleaser::SolarMutexReleaser() Line 1479
at c:\lo\src\core2\include\vcl\svapp.hxx(1479)
fps.dll!fpicker::win32::vista::AsyncRequests::~AsyncRequests() Line 84
at c:\lo\src\core2\fpicker\source\win32\asyncrequests.cxx(84)
fps.dll!fpicker::win32::vista::VistaFilePicker::~VistaFilePicker() Line 72
at c:\lo\src\core2\fpicker\source\win32\vistafilepicker.cxx(72)
fps.dll!fpicker::win32::vista::VistaFilePicker::`scalar deleting destructor'(unsigned int)
cppuhelper3MSC.dll!cppu::OWeakObject::release() Line 233
at c:\lo\src\core2\cppuhelper\source\weak.cxx(233)
cppuhelper3MSC.dll!cppu::WeakComponentImplHelperBase::release() Line 84
at c:\lo\src\core2\cppuhelper\source\implbase.cxx(84)
fps.dll!cppu::PartialWeakComponentImplHelper<com::sun::star::ui::dialogs::XFilePicker3,com::sun::star::ui::dialogs::XFilePickerControlAccess,com::sun::star::ui::dialogs::XFilePreview,com::sun::star::ui::dialogs::XFolderPicker2,com::sun::star::lang::XInitialization,com::sun::star::lang::XServiceInfo>::release() Line 86
at c:\lo\src\core2\include\cppuhelper\compbase.hxx(86)
invocationlo.dll!com::sun::star::uno::cpp_release(void * pCppI) Line 48
at c:\lo\src\core2\include\com\sun\star\uno\genfunc.hxx(48)
cppu3.dll!cppu::_release(void * p, void(*)(void *) release) Line 86
at c:\lo\src\core2\cppu\source\uno\prim.hxx(86)
cppu3.dll!cppu::_destructAny(_uno_Any * pAny, void(*)(void *) release) Line 130
at c:\lo\src\core2\cppu\source\uno\destr.hxx(130)
cppu3.dll!uno_any_destruct(_uno_Any * pValue, void(*)(void *) release) Line 131
at c:\lo\src\core2\cppu\source\uno\any.cxx(131)
invocationlo.dll!com::sun::star::uno::Any::~Any() Line 111
at c:\lo\src\core2\include\com\sun\star\uno\any.hxx(111)
invocationlo.dll!stoc_inv::Invocation_Impl::~Invocation_Impl()
invocationlo.dll!stoc_inv::Invocation_Impl::`scalar deleting destructor'(unsigned int)
cppuhelper3MSC.dll!cppu::OWeakObject::release() Line 233
at c:\lo\src\core2\cppuhelper\source\weak.cxx(233)
invocationlo.dll!stoc_inv::Invocation_Impl::release() Line 110
at c:\lo\src\core2\stoc\source\invocation\invocation.cxx(110)
pyuno_d.pyd!com::sun::star::uno::Reference<com::sun::star::script::XInvocation2>::~Reference<com::sun::star::script::XInvocation2>() Line 110
at c:\lo\src\core2\include\com\sun\star\uno\reference.hxx(110)
pyuno_d.pyd!pyuno::PyUNOInternals::~PyUNOInternals()
pyuno_d.pyd!pyuno::PyUNOInternals::`scalar deleting destructor'(unsigned int)
pyuno_d.pyd!pyuno::PyUNO_del(_object * self) Line 81
at c:\lo\src\core2\pyuno\source\module\pyuno.cxx(81)
python35_d.dll!_Py_Dealloc(_object * op) Line 1795
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\object.c(1795)
python35_d.dll!dict_dealloc(PyDictObject * mp) Line 1646
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\dictobject.c(1646)
python35_d.dll!_Py_Dealloc(_object * op) Line 1795
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\object.c(1795)
python35_d.dll!subtype_dealloc(_object * self) Line 1194
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\typeobject.c(1194)
python35_d.dll!_Py_Dealloc(_object * op) Line 1795
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\object.c(1795)
python35_d.dll!frame_dealloc(_frame * f) Line 431
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\frameobject.c(431)
python35_d.dll!_Py_Dealloc(_object * op) Line 1795
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\object.c(1795)
python35_d.dll!fast_function(_object * func, _object * * * pp_stack, int n, int na, int nk) Line 4858
at c:\lo\src\core2\workdir\unpackedtarball\python3\python\ceval.c(4858)
python35_d.dll!call_function(_object * * * pp_stack, int oparg) Line 4783
at c:\lo\src\core2\workdir\unpackedtarball\python3\python\ceval.c(4783)
python35_d.dll!PyEval_EvalFrameEx(_frame * f, int throwflag) Line 3291
at c:\lo\src\core2\workdir\unpackedtarball\python3\python\ceval.c(3291)
python35_d.dll!_PyEval_EvalCodeWithName(_object * _co, _object * globals, _object * locals, _object * * args, int argcount, _object * * kws, int kwcount, _object * * defs, int defcount, _object * kwdefs, _object * closure, _object * name, _object * qualname) Line 4080
at c:\lo\src\core2\workdir\unpackedtarball\python3\python\ceval.c(4080)
python35_d.dll!PyEval_EvalCodeEx(_object * _co, _object * globals, _object * locals, _object * * args, int argcount, _object * * kws, int kwcount, _object * * defs, int defcount, _object * kwdefs, _object * closure) Line 4092
at c:\lo\src\core2\workdir\unpackedtarball\python3\python\ceval.c(4092)
python35_d.dll!function_call(_object * func, _object * arg, _object * kw) Line 627
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\funcobject.c(627)
python35_d.dll!PyObject_Call(_object * func, _object * arg, _object * kw) Line 2166
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\abstract.c(2166)
python35_d.dll!method_call(_object * func, _object * arg, _object * kw) Line 330
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\classobject.c(330)
python35_d.dll!PyObject_Call(_object * func, _object * arg, _object * kw) Line 2166
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\abstract.c(2166)
python35_d.dll!PyEval_CallObjectWithKeywords(_object * func, _object * arg, _object * kw) Line 4634
at c:\lo\src\core2\workdir\unpackedtarball\python3\python\ceval.c(4634)
python35_d.dll!PyObject_CallObject(_object * o, _object * a) Line 2092
at c:\lo\src\core2\workdir\unpackedtarball\python3\objects\abstract.c(2092)
pyuno_d.pyd!pyuno::Adapter::invoke(const rtl::OUString & aFunctionName, const com::sun::star::uno::Sequence<com::sun::star::uno::Any> & aParams, com::sun::star::uno::Sequence<short> & aOutParamIndex, com::sun::star::uno::Sequence<com::sun::star::uno::Any> & aOutParam) Line 251
at c:\lo\src\core2\pyuno\source\module\pyuno_adapter.cxx(251)
msci_uno.dll!`anonymous namespace'::callVirtualMethod(void * pAdjustedThisPtr, long nVtableIndex, void * pRegisterReturn, _typelib_TypeClass eReturnTypeClass, long * pStackLongs, long nStackLongs) Line 74
at c:\lo\src\core2\bridges\source\cpp_uno\msvc_win32_intel\uno2cpp.cxx(74)
msci_uno.dll!`anonymous namespace'::cpp_call(bridges::cpp_uno::shared::UnoInterfaceProxy * pThis, bridges::cpp_uno::shared::VtableSlot aVtableSlot, _typelib_TypeDescriptionReference * pReturnTypeRef, long nParams, _typelib_MethodParameter * pParams, void * pUnoReturn, void * * pUnoArgs, _uno_Any * * ppUnoExc) Line 249
at c:\lo\src\core2\bridges\source\cpp_uno\msvc_win32_intel\uno2cpp.cxx(249)
msci_uno.dll!unoInterfaceProxyDispatch(_uno_Interface * pUnoI, const _typelib_TypeDescription * pMemberDescr, void * pReturn, void * * pArgs, _uno_Any * * ppException) Line 430
at c:\lo\src\core2\bridges\source\cpp_uno\msvc_win32_intel\uno2cpp.cxx(430)
invocadaptlo.dll!stoc_invadp::AdapterImpl::invoke(const _typelib_TypeDescription * pMemberType, void * pReturn, void * * pArgs, _uno_Any * * ppException) Line 467
at c:\lo\src\core2\stoc\source\invocation_adapterfactory\iafactory.cxx(467)
invocadaptlo.dll!adapter_dispatch(_uno_Interface * pUnoI, const _typelib_TypeDescription * pMemberType, void * pReturn, void * * pArgs, _uno_Any * * ppException) Line 617
at c:\lo\src\core2\stoc\source\invocation_adapterfactory\iafactory.cxx(617)
msci_uno.dll!`anonymous namespace'::cpp2uno_call(bridges::cpp_uno::shared::CppInterfaceProxy * pThis, const _typelib_TypeDescription * pMemberTypeDescr, _typelib_TypeDescriptionReference * pReturnTypeRef, long nParams, _typelib_MethodParameter * pParams, void * * pCallStack, __int64 * pRegisterReturn) Line 155
at c:\lo\src\core2\bridges\source\cpp_uno\msvc_win32_intel\cpp2uno.cxx(155)
msci_uno.dll!`anonymous namespace'::cpp_mediate(void * * pCallStack, long nFunctionIndex, long nVtableOffset, __int64 * pRegisterReturn) Line 345
at c:\lo\src\core2\bridges\source\cpp_uno\msvc_win32_intel\cpp2uno.cxx(345)
msci_uno.dll!`anonymous namespace'::cpp_vtable_call() Line 380
at c:\lo\src\core2\bridges\source\cpp_uno\msvc_win32_intel\cpp2uno.cxx(380)
tklo.dll!ActionListenerMultiplexer::actionPerformed(const com::sun::star::awt::ActionEvent & evt) Line 146
at c:\lo\src\core2\toolkit\source\helper\listenermultiplexer.cxx(146)
tklo.dll!ActionListenerMultiplexer::actionPerformed(const com::sun::star::awt::ActionEvent & evt) Line 146
at c:\lo\src\core2\toolkit\source\helper\listenermultiplexer.cxx(146)
tklo.dll!VCLXButton::ProcessWindowEvent::__l8::<lambda>() Line 593
at c:\lo\src\core2\toolkit\source\awt\vclxwindows.cxx(593)
tklo.dll!std::_Invoker_functor::_Call<void <lambda>(void) &>(VCLXButton::ProcessWindowEvent::__l8::void <lambda>(void) & _Obj)
tklo.dll!std::invoke<void <lambda>(void) &>(VCLXButton::ProcessWindowEvent::__l8::void <lambda>(void) & _Obj)
tklo.dll!std::_Invoker_ret<void,1>::_Call<void <lambda>(void) &>(VCLXButton::ProcessWindowEvent::__l8::void <lambda>(void) & <_Vals_0>)
tklo.dll!std::_Func_impl_no_alloc<void <lambda>(void),void>::_Do_call()
tklo.dll!std::_Func_class<void>::operator()()
tklo.dll!VCLXWindowImpl::OnProcessCallbacks(void * __formal) Line 300
at c:\lo\src\core2\toolkit\source\awt\vclxwindow.cxx(300)
...

So it seems like the only place where we could guarantee the invariant
to hold solar mutex for GUI protection before releasing it is right
before the releaser itself.

Co-authored-by: Jan-Marek Glogowski <glogow@fbihome.de>

Change-Id: Ie95bd3423a25f210f0bbb2a88288c7c60a528ea0
Reviewed-on: https://gerrit.libreoffice.org/70301
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>

1 files changed, 5 insertions, 0 deletions

diff --git a/fpicker/source/win32/asyncrequests.cxx b/fpicker/source/win32/asyncrequests.cxx
index 79a26f894437..665c0d1d6dc7 100644
--- a/fpicker/source/win32/asyncrequests.cxx
+++ b/fpicker/source/win32/asyncrequests.cxx
@@ -80,6 +80,11 @@ AsyncRequests::~AsyncRequests()
     // the already destructed SolarMutex, which would crash LO on exit.
     if (isRunning())
     {
+        // tdf#123502: make sure we actually hold the mutex before releasing it
+        // UNO directly destroys the VistaFilePicker object, so we need GUI protection in there.
+        // But since we redirect GUI stuff to the async thread we also have to release it, so we
+        // can join it, if the thread currently blocks on the SolarMutex.
+        SolarMutexGuard aGuard;
         SolarMutexReleaser aReleaser;
         join();
     }