Improve safety in code (#588)

Delete areas later so that that they can be accessed by
(inherited) dock widgets in dtor. Add some QPointer to
prevent crashes.

Hence allow users to do more while dock widgets etc
are being destroyed.
This commit is contained in:
tmartsum 2023-12-08 06:47:02 +01:00 committed by GitHub
parent 6c98c29855
commit f848df74c3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 69 additions and 37 deletions

View File

@ -756,7 +756,7 @@ int CDockAreaWidget::openDockWidgetsCount() const
int Count = 0; int Count = 0;
for (int i = 0; i < d->ContentsLayout->count(); ++i) for (int i = 0; i < d->ContentsLayout->count(); ++i)
{ {
if (!dockWidget(i)->isClosed()) if (dockWidget(i) && !dockWidget(i)->isClosed())
{ {
++Count; ++Count;
} }
@ -772,7 +772,7 @@ QList<CDockWidget*> CDockAreaWidget::openedDockWidgets() const
for (int i = 0; i < d->ContentsLayout->count(); ++i) for (int i = 0; i < d->ContentsLayout->count(); ++i)
{ {
CDockWidget* DockWidget = dockWidget(i); CDockWidget* DockWidget = dockWidget(i);
if (!DockWidget->isClosed()) if (DockWidget && !DockWidget->isClosed())
{ {
DockWidgetList.append(dockWidget(i)); DockWidgetList.append(dockWidget(i));
} }
@ -786,7 +786,7 @@ int CDockAreaWidget::indexOfFirstOpenDockWidget() const
{ {
for (int i = 0; i < d->ContentsLayout->count(); ++i) for (int i = 0; i < d->ContentsLayout->count(); ++i)
{ {
if (!dockWidget(i)->isClosed()) if (dockWidget(i) && !dockWidget(i)->isClosed())
{ {
return i; return i;
} }
@ -809,7 +809,6 @@ CDockWidget* CDockAreaWidget::dockWidget(int Index) const
return qobject_cast<CDockWidget*>(d->ContentsLayout->widget(Index)); return qobject_cast<CDockWidget*>(d->ContentsLayout->widget(Index));
} }
//============================================================================ //============================================================================
void CDockAreaWidget::reorderDockWidget(int fromIndex, int toIndex) void CDockAreaWidget::reorderDockWidget(int fromIndex, int toIndex)
{ {

View File

@ -141,7 +141,7 @@ public:
CDockContainerWidget* _this; CDockContainerWidget* _this;
QPointer<CDockManager> DockManager; QPointer<CDockManager> DockManager;
unsigned int zOrderIndex = 0; unsigned int zOrderIndex = 0;
QList<CDockAreaWidget*> DockAreas; QList<QPointer<CDockAreaWidget>> DockAreas;
QList<CAutoHideDockContainer*> AutoHideWidgets; QList<CAutoHideDockContainer*> AutoHideWidgets;
QMap<SideBarLocation, CAutoHideSideBar*> SideTabBarWidgets; QMap<SideBarLocation, CAutoHideSideBar*> SideTabBarWidgets;
QGridLayout* Layout = nullptr; QGridLayout* Layout = nullptr;
@ -299,7 +299,11 @@ public:
VisibleDockAreaCount = 0; VisibleDockAreaCount = 0;
for (auto DockArea : DockAreas) for (auto DockArea : DockAreas)
{ {
VisibleDockAreaCount += DockArea->isHidden() ? 0 : 1; if (!DockArea)
{
continue;
}
VisibleDockAreaCount += (DockArea->isHidden() ? 0 : 1);
} }
} }
@ -924,7 +928,10 @@ void DockContainerWidgetPrivate::addDockAreasToList(const QList<CDockAreaWidget*
//============================================================================ //============================================================================
void DockContainerWidgetPrivate::appendDockAreas(const QList<CDockAreaWidget*> NewDockAreas) void DockContainerWidgetPrivate::appendDockAreas(const QList<CDockAreaWidget*> NewDockAreas)
{ {
DockAreas.append(NewDockAreas); for (auto *newDockArea : NewDockAreas)
{
DockAreas.append(newDockArea);
}
for (auto DockArea : NewDockAreas) for (auto DockArea : NewDockAreas)
{ {
QObject::connect(DockArea, QObject::connect(DockArea,
@ -1641,7 +1648,7 @@ CDockAreaWidget* CDockContainerWidget::dockAreaAt(const QPoint& GlobalPos) const
{ {
for (const auto& DockArea : d->DockAreas) for (const auto& DockArea : d->DockAreas)
{ {
if (DockArea->isVisible() && DockArea->rect().contains(DockArea->mapFromGlobal(GlobalPos))) if (DockArea && DockArea->isVisible() && DockArea->rect().contains(DockArea->mapFromGlobal(GlobalPos)))
{ {
return DockArea; return DockArea;
} }
@ -1678,7 +1685,7 @@ int CDockContainerWidget::visibleDockAreaCount() const
int Result = 0; int Result = 0;
for (auto DockArea : d->DockAreas) for (auto DockArea : d->DockAreas)
{ {
Result += DockArea->isHidden() ? 0 : 1; Result += (!DockArea || DockArea->isHidden()) ? 0 : 1;
} }
return Result; return Result;
@ -1802,7 +1809,7 @@ QList<CDockAreaWidget*> CDockContainerWidget::openedDockAreas() const
QList<CDockAreaWidget*> Result; QList<CDockAreaWidget*> Result;
for (auto DockArea : d->DockAreas) for (auto DockArea : d->DockAreas)
{ {
if (!DockArea->isHidden()) if (DockArea && !DockArea->isHidden())
{ {
Result.append(DockArea); Result.append(DockArea);
} }
@ -1818,7 +1825,7 @@ QList<CDockWidget*> CDockContainerWidget::openedDockWidgets() const
QList<CDockWidget*> DockWidgetList; QList<CDockWidget*> DockWidgetList;
for (auto DockArea : d->DockAreas) for (auto DockArea : d->DockAreas)
{ {
if (!DockArea->isHidden()) if (DockArea && !DockArea->isHidden())
{ {
DockWidgetList.append(DockArea->openedDockWidgets()); DockWidgetList.append(DockArea->openedDockWidgets());
} }
@ -1833,7 +1840,7 @@ bool CDockContainerWidget::hasOpenDockAreas() const
{ {
for (auto DockArea : d->DockAreas) for (auto DockArea : d->DockAreas)
{ {
if (!DockArea->isHidden()) if (DockArea && !DockArea->isHidden())
{ {
return true; return true;
} }
@ -2058,6 +2065,10 @@ QList<CDockWidget*> CDockContainerWidget::dockWidgets() const
QList<CDockWidget*> Result; QList<CDockWidget*> Result;
for (const auto DockArea : d->DockAreas) for (const auto DockArea : d->DockAreas)
{ {
if (!DockArea)
{
continue;
}
Result.append(DockArea->dockWidgets()); Result.append(DockArea->dockWidgets());
} }
@ -2090,6 +2101,10 @@ CDockWidget::DockWidgetFeatures CDockContainerWidget::features() const
CDockWidget::DockWidgetFeatures Features(CDockWidget::AllDockWidgetFeatures); CDockWidget::DockWidgetFeatures Features(CDockWidget::AllDockWidgetFeatures);
for (const auto DockArea : d->DockAreas) for (const auto DockArea : d->DockAreas)
{ {
if (!DockArea)
{
continue;
}
Features &= DockArea->features(); Features &= DockArea->features();
} }
@ -2109,7 +2124,7 @@ void CDockContainerWidget::closeOtherAreas(CDockAreaWidget* KeepOpenArea)
{ {
for (const auto DockArea : d->DockAreas) for (const auto DockArea : d->DockAreas)
{ {
if (DockArea == KeepOpenArea) if (!DockArea || DockArea == KeepOpenArea)
{ {
continue; continue;
} }

View File

@ -115,6 +115,7 @@ DockFocusControllerPrivate::DockFocusControllerPrivate(
//============================================================================ //============================================================================
void DockFocusControllerPrivate::updateDockWidgetFocus(CDockWidget* DockWidget) void DockFocusControllerPrivate::updateDockWidgetFocus(CDockWidget* DockWidget)
{ {
if (!DockWidget) return;
if (!DockWidget->features().testFlag(CDockWidget::DockWidgetFocusable)) if (!DockWidget->features().testFlag(CDockWidget::DockWidgetFocusable))
{ {
return; return;

View File

@ -103,8 +103,8 @@ static QString FloatingContainersTitle;
struct DockManagerPrivate struct DockManagerPrivate
{ {
CDockManager* _this; CDockManager* _this;
QList<CFloatingDockContainer*> FloatingWidgets; QList<QPointer<CFloatingDockContainer>> FloatingWidgets;
QList<CFloatingDockContainer*> HiddenFloatingWidgets; QList<QPointer<CFloatingDockContainer>> HiddenFloatingWidgets;
QList<CDockContainerWidget*> Containers; QList<CDockContainerWidget*> Containers;
CDockOverlay* ContainerOverlay; CDockOverlay* ContainerOverlay;
CDockOverlay* DockAreaOverlay; CDockOverlay* DockAreaOverlay;
@ -153,7 +153,10 @@ struct DockManagerPrivate
// Hide updates of floating widgets from user // Hide updates of floating widgets from user
for (auto FloatingWidget : FloatingWidgets) for (auto FloatingWidget : FloatingWidgets)
{ {
FloatingWidget->hide(); if (FloatingWidget)
{
FloatingWidget->hide();
}
} }
} }
@ -333,7 +336,8 @@ bool DockManagerPrivate::restoreStateFromXml(const QByteArray &state, int versi
int FloatingWidgetIndex = DockContainerCount - 1; int FloatingWidgetIndex = DockContainerCount - 1;
for (int i = FloatingWidgetIndex; i < FloatingWidgets.count(); ++i) for (int i = FloatingWidgetIndex; i < FloatingWidgets.count(); ++i)
{ {
auto* floatingWidget = FloatingWidgets[i]; CFloatingDockContainer* floatingWidget = FloatingWidgets[i];
if (!floatingWidget) continue;
_this->removeDockContainer(floatingWidget->dockContainer()); _this->removeDockContainer(floatingWidget->dockContainer());
floatingWidget->deleteLater(); floatingWidget->deleteLater();
} }
@ -536,25 +540,27 @@ CDockManager::CDockManager(QWidget *parent) :
CDockManager::~CDockManager() CDockManager::~CDockManager()
{ {
// fix memory leaks, see https://github.com/githubuser0xFFFF/Qt-Advanced-Docking-System/issues/307 // fix memory leaks, see https://github.com/githubuser0xFFFF/Qt-Advanced-Docking-System/issues/307
std::vector<ads::CDockAreaWidget*> areas; std::vector<QPointer<ads::CDockAreaWidget>> areas;
for ( int i = 0; i != dockAreaCount(); ++i ) for (int i = 0; i != dockAreaCount(); ++i)
{ {
auto area = dockArea(i); areas.push_back( dockArea(i) );
if ( area->dockManager() == this ) }
areas.push_back( area ); for ( auto area : areas )
// else, this is surprising, looks like this CDockAreaWidget is child of two different CDockManager {
// this is reproductible by https://github.com/githubuser0xFFFF/Qt-Advanced-Docking-System/issues/585 testcase if (!area || area->dockManager() != this) continue;
// then, when a CDockManager deletes itself, it deletes the CDockAreaWidget, then, the other
// CDockManager will try to delete the CDockAreaWidget again and this will crash
// So let's just delete CDockAreaWidget we are the parent of!
}
for ( auto area : areas )
{
for ( auto widget : area->dockWidgets() )
delete widget;
delete area; // QPointer delete safety - just in case some dock wigdet in destruction
} // deletes another related/twin or child dock widget.
std::vector<QPointer<QWidget>> deleteWidgets;
for ( auto widget : area->dockWidgets() )
{
deleteWidgets.push_back(widget);
}
for ( auto ptrWdg : deleteWidgets)
{
delete ptrWdg;
}
}
auto FloatingWidgets = d->FloatingWidgets; auto FloatingWidgets = d->FloatingWidgets;
for (auto FloatingWidget : FloatingWidgets) for (auto FloatingWidget : FloatingWidgets)
@ -562,6 +568,12 @@ CDockManager::~CDockManager()
delete FloatingWidget; delete FloatingWidget;
} }
// Delete Dock Widgets before Areas so widgets can access them late (like dtor)
for ( auto area : areas )
{
delete area;
}
delete d; delete d;
} }
@ -732,7 +744,12 @@ const QList<CDockContainerWidget*> CDockManager::dockContainers() const
//============================================================================ //============================================================================
const QList<CFloatingDockContainer*> CDockManager::floatingWidgets() const const QList<CFloatingDockContainer*> CDockManager::floatingWidgets() const
{ {
return d->FloatingWidgets; QList<CFloatingDockContainer*> res;
for (auto &fl : d->FloatingWidgets)
{
if (fl) res.append(fl);
}
return res;
} }

View File

@ -81,7 +81,7 @@ struct DockWidgetPrivate
CDockWidgetTab* TabWidget = nullptr; CDockWidgetTab* TabWidget = nullptr;
CDockWidget::DockWidgetFeatures Features = CDockWidget::DefaultDockWidgetFeatures; CDockWidget::DockWidgetFeatures Features = CDockWidget::DefaultDockWidgetFeatures;
CDockManager* DockManager = nullptr; CDockManager* DockManager = nullptr;
CDockAreaWidget* DockArea = nullptr; QPointer<CDockAreaWidget> DockArea;
QAction* ToggleViewAction = nullptr; QAction* ToggleViewAction = nullptr;
bool Closed = false; bool Closed = false;
QScrollArea* ScrollArea = nullptr; QScrollArea* ScrollArea = nullptr;