dbaccess/source/ui/querydesign/QueryTableView.cxx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
New commits: commit 3a681cf1498da6ffba430cd06192290b8c5e2251 Author: Neil Roberts <[email protected]> AuthorDate: Mon Nov 10 17:41:46 2025 +0100 Commit: Adolfo Jayme Barrientos <[email protected]> CommitDate: Tue Nov 11 10:05:50 2025 +0100 tdf#99619 Don’t add undo action when removing conn but not deleting Previously when undo is invoked to undo adding a connection between two tables OQueryTableView::RemoveConnection gets invoked which would end up trying to add another undo action which would re-add the connection when invoked. Because this happens during an undo invocation, the undo manager is disabled and instead the new undo action gets destroyed as soon as the first undo invocation completes. However, because the new action is a delete it tries to become the owner of the deleted connection. That means that when it’s destroyed it will call dispose on the connection. However, the original undo action also tries to become the owner of the connection even though it is now disposed so that it can put the connection back if redo is invoked on the action. In that case the first undo action would try to put back a connection which is already disposed which ultimately leads to a crash. In order to fix this, the RemoveConnection implementation now avoids adding the undo action if the bDelete parameter is false. As far as I can tell from inspecting the code the only way this can happen is if RemoveConnection is invoked from OQueryTableView::DropConnection and that only happens when applying the undo/redo actions. I think it semantically makes sense not to make an undo action for the removal when bDelete is false because the undo action assumes the connection is either currently linked to a table or it itself is the owner, which means that it will ultimately cause the connection to be deleted. If the connection being removed shouldn’t be deleted then it implies there is already another owner. Change-Id: I69976c7fa8f035f9d1613382e7d0698311cf4b71 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193761 Reviewed-by: Noel Grandin <[email protected]> Tested-by: Jenkins (cherry picked from commit 2dde26dcaee48d37a508617515addce634d0c96c) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193780 Reviewed-by: Adolfo Jayme Barrientos <[email protected]> diff --git a/dbaccess/source/ui/querydesign/QueryTableView.cxx b/dbaccess/source/ui/querydesign/QueryTableView.cxx index 2ca8c71b837f..f6a588a0e7e8 100644 --- a/dbaccess/source/ui/querydesign/QueryTableView.cxx +++ b/dbaccess/source/ui/querydesign/QueryTableView.cxx @@ -607,18 +607,22 @@ void OQueryTableView::createNewConnection() SelectConn( pConn ); } -bool OQueryTableView::RemoveConnection(VclPtr<OTableConnection>& rConnection, bool /*_bDelete*/) +bool OQueryTableView::RemoveConnection(VclPtr<OTableConnection>& rConnection, bool bDelete) { VclPtr<OQueryTableConnection> xConnection(static_cast<OQueryTableConnection*>(rConnection.get())); // we don't want that our connection will be deleted, we put it in the undo manager bool bRet = OJoinTableView::RemoveConnection(rConnection, false); - // add undo action - addUndoAction(this, - std::make_unique<OQueryDelTabConnUndoAction>(this), - xConnection.get(), - true); + // Don’t add an undo action if the connection isn’t to be deleted because the action will + // effectively take ownership of it and end up deleting it. See tdf#99619 + if (bDelete) + { + addUndoAction(this, + std::make_unique<OQueryDelTabConnUndoAction>(this), + xConnection.get(), + true); + } return bRet; }
