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 <[email protected]>
This commit is contained in:
Andrew Lindesay
2020-05-24 15:51:40 +00:00
committed by waddlesplash
parent 51dd385e3e
commit f95ec23e95
4 changed files with 21 additions and 19 deletions
@@ -36,7 +36,7 @@ void
AbstractProcess::SetListener(AbstractProcessListener* listener) AbstractProcess::SetListener(AbstractProcessListener* listener)
{ {
AutoLocker<BLocker> locker(&fLock); AutoLocker<BLocker> locker(&fLock);
fListener = listener; fListener = BReference<AbstractProcessListener>(listener);
} }
@@ -64,7 +64,7 @@ AbstractProcess::Run()
if (runResult != B_OK) if (runResult != B_OK)
printf("[%s] an error has arisen; %s\n", Name(), strerror(runResult)); printf("[%s] an error has arisen; %s\n", Name(), strerror(runResult));
AbstractProcessListener* listener; BReference<AbstractProcessListener> listener;
{ {
AutoLocker<BLocker> locker(&fLock); AutoLocker<BLocker> locker(&fLock);
@@ -76,7 +76,7 @@ AbstractProcess::Run()
// this process may be part of a larger bulk-load process and // this process may be part of a larger bulk-load process and
// if so, the process orchestration needs to know when this // if so, the process orchestration needs to know when this
// process has completed. // process has completed.
if (listener != NULL) if (listener.Get() != NULL)
listener->ProcessExited(); listener->ProcessExited();
return runResult; return runResult;
@@ -110,7 +110,7 @@ status_t
AbstractProcess::Stop() AbstractProcess::Stop()
{ {
status_t result = B_CANCELED; status_t result = B_CANCELED;
AbstractProcessListener* listener = NULL; BReference<AbstractProcessListener> listener = NULL;
{ {
AutoLocker<BLocker> locker(&fLock); AutoLocker<BLocker> locker(&fLock);
@@ -126,7 +126,7 @@ AbstractProcess::Stop()
} }
} }
if (listener != NULL) if (listener.Get() != NULL)
listener->ProcessExited(); listener->ProcessExited();
return result; return result;
+3 -2
View File
@@ -8,6 +8,7 @@
#define ABSTRACT_PROCESS_H #define ABSTRACT_PROCESS_H
#include <String.h> #include <String.h>
#include <Referenceable.h>
#include <Url.h> #include <Url.h>
#include "StandardMetaData.h" #include "StandardMetaData.h"
@@ -26,7 +27,7 @@ typedef enum process_state {
failure. failure.
*/ */
class AbstractProcessListener { class AbstractProcessListener : public BReferenceable {
public: public:
virtual void ProcessExited() = 0; virtual void ProcessExited() = 0;
}; };
@@ -55,7 +56,7 @@ protected:
private: private:
BLocker fLock; BLocker fLock;
AbstractProcessListener* BReference<AbstractProcessListener>
fListener; fListener;
bool fWasStopped; bool fWasStopped;
process_state fProcessState; process_state fProcessState;
+10 -10
View File
@@ -496,7 +496,7 @@ MainWindow::MessageReceived(BMessage* message)
} }
_AddRemovePackageFromLists(ref); _AddRemovePackageFromLists(ref);
if ((changes & PKG_CHANGED_STATE) != 0 if ((changes & PKG_CHANGED_STATE) != 0
&& fCoordinator == NULL) { && fCoordinator.Get() == NULL) {
fWorkStatusView->PackageStatusChanged(ref); fWorkStatusView->PackageStatusChanged(ref);
} }
} }
@@ -1334,14 +1334,14 @@ MainWindow::_AddProcessCoordinator(ProcessCoordinator* item)
{ {
AutoLocker<BLocker> lock(&fCoordinatorLock); AutoLocker<BLocker> lock(&fCoordinatorLock);
if (fCoordinator == NULL) { if (fCoordinator.Get() == NULL) {
if (acquire_sem(fCoordinatorRunningSem) != B_OK) if (acquire_sem(fCoordinatorRunningSem) != B_OK)
debugger("unable to acquire the process coordinator sem"); debugger("unable to acquire the process coordinator sem");
if (Logger::IsInfoEnabled()) { if (Logger::IsInfoEnabled()) {
printf("adding and starting a process coordinator [%s]\n", printf("adding and starting a process coordinator [%s]\n",
item->Name().String()); item->Name().String());
} }
fCoordinator = item; fCoordinator = BReference<ProcessCoordinator>(item);
fCoordinator->Start(); fCoordinator->Start();
} }
else { else {
@@ -1364,7 +1364,7 @@ MainWindow::_SpinUntilProcessCoordinatorComplete()
debugger("unable to release the process coordinator sem"); debugger("unable to release the process coordinator sem");
{ {
AutoLocker<BLocker> lock(&fCoordinatorLock); AutoLocker<BLocker> lock(&fCoordinatorLock);
if (fCoordinator == NULL) if (fCoordinator.Get() == NULL)
return; return;
} }
} }
@@ -1381,16 +1381,15 @@ MainWindow::_StopProcessCoordinators()
AutoLocker<BLocker> lock(&fCoordinatorLock); AutoLocker<BLocker> lock(&fCoordinatorLock);
while (!fCoordinatorQueue.empty()) { while (!fCoordinatorQueue.empty()) {
ProcessCoordinator *processCoordinator = fCoordinatorQueue.front(); BReference<ProcessCoordinator> processCoordinator = fCoordinatorQueue.front();
if (Logger::IsInfoEnabled()) { if (Logger::IsInfoEnabled()) {
printf("will drop queued process coordinator [%s]\n", printf("will drop queued process coordinator [%s]\n",
processCoordinator->Name().String()); processCoordinator->Name().String());
} }
fCoordinatorQueue.pop(); fCoordinatorQueue.pop();
delete processCoordinator;
} }
if (fCoordinator != NULL) { if (fCoordinator.Get() != NULL) {
fCoordinator->Stop(); fCoordinator->Stop();
} }
} }
@@ -1416,7 +1415,7 @@ MainWindow::CoordinatorChanged(ProcessCoordinatorState& coordinatorState)
{ {
AutoLocker<BLocker> lock(&fCoordinatorLock); AutoLocker<BLocker> lock(&fCoordinatorLock);
if (fCoordinator == coordinatorState.Coordinator()) { if (fCoordinator.Get() == coordinatorState.Coordinator()) {
if (!coordinatorState.IsRunning()) { if (!coordinatorState.IsRunning()) {
if (release_sem(fCoordinatorRunningSem) != B_OK) if (release_sem(fCoordinatorRunningSem) != B_OK)
debugger("unable to release the process coordinator sem"); debugger("unable to release the process coordinator sem");
@@ -1434,8 +1433,9 @@ MainWindow::CoordinatorChanged(ProcessCoordinatorState& coordinatorState)
messenger.SendMessage(message); messenger.SendMessage(message);
} }
delete fCoordinator; fCoordinator = BReference<ProcessCoordinator>(NULL);
fCoordinator = NULL; // will delete the old process coordinator if it is not used
// elsewhere.
// now schedule the next one. // now schedule the next one.
if (!fCoordinatorQueue.empty()) { if (!fCoordinatorQueue.empty()) {
+3 -2
View File
@@ -155,9 +155,10 @@ private:
Model fModel; Model fModel;
ModelListenerRef fModelListener; ModelListenerRef fModelListener;
std::queue<ProcessCoordinator*> std::queue<BReference<ProcessCoordinator>>
fCoordinatorQueue; fCoordinatorQueue;
ProcessCoordinator* fCoordinator; BReference<ProcessCoordinator>
fCoordinator;
BLocker fCoordinatorLock; BLocker fCoordinatorLock;
sem_id fCoordinatorRunningSem; sem_id fCoordinatorRunningSem;