vcl/source/window/dialog.cxx | 47 +++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 26 deletions(-)
New commits: commit e4d1a567e984dbece8ff9e0359ed5c8e14654468 Author: Noel Grandin <[email protected]> AuthorDate: Fri Jan 31 10:45:31 2025 +0200 Commit: Andras Timar <[email protected]> CommitDate: Thu Mar 5 10:54:30 2026 +0100 make Dialog destruction a little safer rather keep the Impl object around for the lifetime of the Dialog object. where I saw a backtrace that looked like vcl/source/window/dialog.cxx:1098: short Dialog::Execute(): Assertion `mpDialogImpl' failed. Dialog::Execute() SalInstanceDialog::run() ScSpellingEngine::ShowFinishDialog() ScConversionEngineBase::FindNextConversionCell() ScSpellingEngine::SpellNextDocument() ScSpellDialogChildWindow::GetNextWrongSentence(bool) Change-Id: I5858f3cd126185bc4f59932dfa53f6a6a3a66aa6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/180980 Reviewed-by: Noel Grandin <[email protected]> Tested-by: Jenkins diff --git a/vcl/source/window/dialog.cxx b/vcl/source/window/dialog.cxx index 83a8e8babaf2..6a662ef90167 100644 --- a/vcl/source/window/dialog.cxx +++ b/vcl/source/window/dialog.cxx @@ -372,6 +372,11 @@ struct DialogImpl #endif ~DialogImpl() + { + disposeAndClear(); + } + + void disposeAndClear() { for (VclPtr<PushButton> & pOwnedButton : maOwnedButtons) pOwnedButton.disposeAndClear(); @@ -622,7 +627,7 @@ void Dialog::dispose() { bool bTunnelingEnabled = mpDialogImpl->m_bLOKTunneling; - mpDialogImpl.reset(); + mpDialogImpl->disposeAndClear(); RemoveFromDlgList(); mpActionArea.clear(); mpContentArea.clear(); @@ -1088,20 +1093,11 @@ short Dialog::Execute() OSL_FAIL( "Dialog::Execute() - Dialog destroyed in Execute()" ); } - assert(mpDialogImpl); - if (mpDialogImpl) - { - tools::Long nRet = mpDialogImpl->mnResult; - mpDialogImpl->mnResult = -1; + tools::Long nRet = mpDialogImpl->mnResult; + mpDialogImpl->mnResult = -1; - return static_cast<short>(nRet); - } - else - { - SAL_WARN( "vcl", "Dialog::Execute() : missing mpDialogImpl " ); - return 0; - } + return static_cast<short>(nRet); } // virtual @@ -1136,6 +1132,9 @@ void Dialog::EndDialog( tools::Long nResult ) if (!mbInExecute || isDisposed()) return; + // do not let this object be destroyed from underneath us + VclPtr<vcl::Window> xWindow = this; + const bool bModal = GetType() != WindowType::MODELESSDIALOG; Hide(); @@ -1179,8 +1178,7 @@ void Dialog::EndDialog( tools::Long nResult ) if ( mpDialogImpl->mbStartedModal ) ImplEndExecuteModal(); - // coverity[check_after_deref] - ImplEndExecuteModal might trigger destruction of mpDialogImpl - if ( mpDialogImpl && mpDialogImpl->maEndCtx.isSet() ) + if ( mpDialogImpl->maEndCtx.isSet() ) { auto fn = std::move(mpDialogImpl->maEndCtx.maEndDialogFn); // std::move leaves maEndDialogFn in a valid state with unspecified @@ -1192,22 +1190,19 @@ void Dialog::EndDialog( tools::Long nResult ) fn(nResult); } - if ( mpDialogImpl && mpDialogImpl->mbStartedModal ) + if ( mpDialogImpl->mbStartedModal ) { mpDialogImpl->mbStartedModal = false; mpDialogImpl->mnResult = -1; } mbInExecute = false; - if ( mpDialogImpl ) - { - // Destroy ourselves (if we have a context with VclPtr owner) - std::shared_ptr<weld::DialogController> xOwnerDialogController = std::move(mpDialogImpl->maEndCtx.mxOwnerDialogController); - std::shared_ptr<weld::Dialog> xOwnerSelf = std::move(mpDialogImpl->maEndCtx.mxOwnerSelf); - mpDialogImpl->maEndCtx.mxOwner.disposeAndClear(); - xOwnerDialogController.reset(); - xOwnerSelf.reset(); - } + // Destroy ourselves (if we have a context with VclPtr owner) + std::shared_ptr<weld::DialogController> xOwnerDialogController = std::move(mpDialogImpl->maEndCtx.mxOwnerDialogController); + std::shared_ptr<weld::Dialog> xOwnerSelf = std::move(mpDialogImpl->maEndCtx.mxOwnerSelf); + mpDialogImpl->maEndCtx.mxOwner.disposeAndClear(); + xOwnerDialogController.reset(); + xOwnerSelf.reset(); } namespace vcl @@ -1638,7 +1633,7 @@ void Dialog::Activate() void Dialog::Command(const CommandEvent& rCEvt) { - if (mpDialogImpl && mpDialogImpl->m_aPopupMenuHdl.Call(rCEvt)) + if (mpDialogImpl->m_aPopupMenuHdl.Call(rCEvt)) return; SystemWindow::Command(rCEvt); }
