From f95ec23e9582b09d2067c39903ea8cbbe7bf1e94 Mon Sep 17 00:00:00 2001 From: Andrew Lindesay Date: Sun, 24 May 2020 23:20:39 +1200 Subject: [PATCH] HaikuDepot: Fix Crash on Quit During Load If the system is currently loading-up and populating data and the user quits then it was crashing because of a call to a deleted ProcessCoordinator object. This change implements the reference as BReference ensuring that the ProcessCoordinator object is only deleted after it is not used anywhere. Resolves #16109 Change-Id: If535c151819da37d502283af3745e4148da69026 Reviewed-on: https://review.haiku-os.org/c/haiku/+/2797 Reviewed-by: waddlesplash --- .../haikudepot/server/AbstractProcess.cpp | 10 +++++----- src/apps/haikudepot/server/AbstractProcess.h | 5 +++-- src/apps/haikudepot/ui/MainWindow.cpp | 20 +++++++++---------- src/apps/haikudepot/ui/MainWindow.h | 5 +++-- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/apps/haikudepot/server/AbstractProcess.cpp b/src/apps/haikudepot/server/AbstractProcess.cpp index bc74cedf73..2316f509b1 100644 --- a/src/apps/haikudepot/server/AbstractProcess.cpp +++ b/src/apps/haikudepot/server/AbstractProcess.cpp @@ -36,7 +36,7 @@ void AbstractProcess::SetListener(AbstractProcessListener* listener) { AutoLocker locker(&fLock); - fListener = listener; + fListener = BReference(listener); } @@ -64,7 +64,7 @@ AbstractProcess::Run() if (runResult != B_OK) printf("[%s] an error has arisen; %s\n", Name(), strerror(runResult)); - AbstractProcessListener* listener; + BReference listener; { AutoLocker locker(&fLock); @@ -76,7 +76,7 @@ AbstractProcess::Run() // this process may be part of a larger bulk-load process and // if so, the process orchestration needs to know when this // process has completed. - if (listener != NULL) + if (listener.Get() != NULL) listener->ProcessExited(); return runResult; @@ -110,7 +110,7 @@ status_t AbstractProcess::Stop() { status_t result = B_CANCELED; - AbstractProcessListener* listener = NULL; + BReference listener = NULL; { AutoLocker locker(&fLock); @@ -126,7 +126,7 @@ AbstractProcess::Stop() } } - if (listener != NULL) + if (listener.Get() != NULL) listener->ProcessExited(); return result; diff --git a/src/apps/haikudepot/server/AbstractProcess.h b/src/apps/haikudepot/server/AbstractProcess.h index 001ff8724e..72e04da96e 100644 --- a/src/apps/haikudepot/server/AbstractProcess.h +++ b/src/apps/haikudepot/server/AbstractProcess.h @@ -8,6 +8,7 @@ #define ABSTRACT_PROCESS_H #include +#include #include #include "StandardMetaData.h" @@ -26,7 +27,7 @@ typedef enum process_state { failure. */ -class AbstractProcessListener { +class AbstractProcessListener : public BReferenceable { public: virtual void ProcessExited() = 0; }; @@ -55,7 +56,7 @@ protected: private: BLocker fLock; - AbstractProcessListener* + BReference fListener; bool fWasStopped; process_state fProcessState; diff --git a/src/apps/haikudepot/ui/MainWindow.cpp b/src/apps/haikudepot/ui/MainWindow.cpp index c247616679..3bfd4f5eb5 100644 --- a/src/apps/haikudepot/ui/MainWindow.cpp +++ b/src/apps/haikudepot/ui/MainWindow.cpp @@ -496,7 +496,7 @@ MainWindow::MessageReceived(BMessage* message) } _AddRemovePackageFromLists(ref); if ((changes & PKG_CHANGED_STATE) != 0 - && fCoordinator == NULL) { + && fCoordinator.Get() == NULL) { fWorkStatusView->PackageStatusChanged(ref); } } @@ -1334,14 +1334,14 @@ MainWindow::_AddProcessCoordinator(ProcessCoordinator* item) { AutoLocker lock(&fCoordinatorLock); - if (fCoordinator == NULL) { + if (fCoordinator.Get() == NULL) { if (acquire_sem(fCoordinatorRunningSem) != B_OK) debugger("unable to acquire the process coordinator sem"); if (Logger::IsInfoEnabled()) { printf("adding and starting a process coordinator [%s]\n", item->Name().String()); } - fCoordinator = item; + fCoordinator = BReference(item); fCoordinator->Start(); } else { @@ -1364,7 +1364,7 @@ MainWindow::_SpinUntilProcessCoordinatorComplete() debugger("unable to release the process coordinator sem"); { AutoLocker lock(&fCoordinatorLock); - if (fCoordinator == NULL) + if (fCoordinator.Get() == NULL) return; } } @@ -1381,16 +1381,15 @@ MainWindow::_StopProcessCoordinators() AutoLocker lock(&fCoordinatorLock); while (!fCoordinatorQueue.empty()) { - ProcessCoordinator *processCoordinator = fCoordinatorQueue.front(); + BReference processCoordinator = fCoordinatorQueue.front(); if (Logger::IsInfoEnabled()) { printf("will drop queued process coordinator [%s]\n", processCoordinator->Name().String()); } fCoordinatorQueue.pop(); - delete processCoordinator; } - if (fCoordinator != NULL) { + if (fCoordinator.Get() != NULL) { fCoordinator->Stop(); } } @@ -1416,7 +1415,7 @@ MainWindow::CoordinatorChanged(ProcessCoordinatorState& coordinatorState) { AutoLocker lock(&fCoordinatorLock); - if (fCoordinator == coordinatorState.Coordinator()) { + if (fCoordinator.Get() == coordinatorState.Coordinator()) { if (!coordinatorState.IsRunning()) { if (release_sem(fCoordinatorRunningSem) != B_OK) debugger("unable to release the process coordinator sem"); @@ -1434,8 +1433,9 @@ MainWindow::CoordinatorChanged(ProcessCoordinatorState& coordinatorState) messenger.SendMessage(message); } - delete fCoordinator; - fCoordinator = NULL; + fCoordinator = BReference(NULL); + // will delete the old process coordinator if it is not used + // elsewhere. // now schedule the next one. if (!fCoordinatorQueue.empty()) { diff --git a/src/apps/haikudepot/ui/MainWindow.h b/src/apps/haikudepot/ui/MainWindow.h index 32a3717d9c..5ead8e5960 100644 --- a/src/apps/haikudepot/ui/MainWindow.h +++ b/src/apps/haikudepot/ui/MainWindow.h @@ -155,9 +155,10 @@ private: Model fModel; ModelListenerRef fModelListener; - std::queue + std::queue> fCoordinatorQueue; - ProcessCoordinator* fCoordinator; + BReference + fCoordinator; BLocker fCoordinatorLock; sem_id fCoordinatorRunningSem;