From a10cfc4865da9a1a5da0090f0a65dfb62c7010e2 Mon Sep 17 00:00:00 2001 From: Mike Kaganski Date: Fri, 5 Apr 2019 15:24:31 +0300 Subject: 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::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::~Reference() 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 & aParams, com::sun::star::uno::Sequence & aOutParamIndex, com::sun::star::uno::Sequence & 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::() Line 593 at c:\lo\src\core2\toolkit\source\awt\vclxwindows.cxx(593) tklo.dll!std::_Invoker_functor::_Call(void) &>(VCLXButton::ProcessWindowEvent::__l8::void (void) & _Obj) tklo.dll!std::invoke(void) &>(VCLXButton::ProcessWindowEvent::__l8::void (void) & _Obj) tklo.dll!std::_Invoker_ret::_Call(void) &>(VCLXButton::ProcessWindowEvent::__l8::void (void) & <_Vals_0>) tklo.dll!std::_Func_impl_no_alloc(void),void>::_Do_call() tklo.dll!std::_Func_class::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 Change-Id: Ie95bd3423a25f210f0bbb2a88288c7c60a528ea0 Reviewed-on: https://gerrit.libreoffice.org/70301 Reviewed-by: Mike Kaganski Tested-by: Mike Kaganski --- fpicker/source/win32/asyncrequests.cxx | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'fpicker') 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(); } -- cgit