From Andrew Lindesay <apl@xxxxxxxxxxxxxx>:
Andrew Lindesay has uploaded this change for review. (
https://review.haiku-os.org/c/haiku/+/2797 ;)
Change subject: HaikuDepot: Fix Crash on Quit During Load
......................................................................
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
---
M src/apps/haikudepot/server/AbstractProcess.cpp
M src/apps/haikudepot/server/AbstractProcess.h
M src/apps/haikudepot/ui/MainWindow.cpp
M src/apps/haikudepot/ui/MainWindow.h
4 files changed, 21 insertions(+), 19 deletions(-)
git pull ssh://git.haiku-os.org:22/haiku refs/changes/97/2797/1
diff --git a/src/apps/haikudepot/server/AbstractProcess.cpp
b/src/apps/haikudepot/server/AbstractProcess.cpp
index bc74ced..2316f50 100644
--- a/src/apps/haikudepot/server/AbstractProcess.cpp
+++ b/src/apps/haikudepot/server/AbstractProcess.cpp
@@ -36,7 +36,7 @@
AbstractProcess::SetListener(AbstractProcessListener* listener)
{
AutoLocker<BLocker> locker(&fLock);
- fListener = listener;
+ fListener = BReference<AbstractProcessListener>(listener);
}
@@ -64,7 +64,7 @@
if (runResult != B_OK)
printf("[%s] an error has arisen; %s\n", Name(),
strerror(runResult));
- AbstractProcessListener* listener;
+ BReference<AbstractProcessListener> listener;
{
AutoLocker<BLocker> locker(&fLock);
@@ -76,7 +76,7 @@
// 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 @@
AbstractProcess::Stop()
{
status_t result = B_CANCELED;
- AbstractProcessListener* listener = NULL;
+ BReference<AbstractProcessListener> listener = NULL;
{
AutoLocker<BLocker> locker(&fLock);
@@ -126,7 +126,7 @@
}
}
- 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 001ff87..72e04da 100644
--- a/src/apps/haikudepot/server/AbstractProcess.h
+++ b/src/apps/haikudepot/server/AbstractProcess.h
@@ -8,6 +8,7 @@
#define ABSTRACT_PROCESS_H
#include <String.h>
+#include <Referenceable.h>
#include <Url.h>
#include "StandardMetaData.h"
@@ -26,7 +27,7 @@
failure.
*/
-class AbstractProcessListener {
+class AbstractProcessListener : public BReferenceable {
public:
virtual void ProcessExited() = 0;
};
@@ -55,7 +56,7 @@
private:
BLocker fLock;
- AbstractProcessListener*
+ BReference<AbstractProcessListener>
fListener;
bool fWasStopped;
process_state fProcessState;
diff --git a/src/apps/haikudepot/ui/MainWindow.cpp
b/src/apps/haikudepot/ui/MainWindow.cpp
index c247616..3bfd4f5 100644
--- a/src/apps/haikudepot/ui/MainWindow.cpp
+++ b/src/apps/haikudepot/ui/MainWindow.cpp
@@ -496,7 +496,7 @@
}
_AddRemovePackageFromLists(ref);
if ((changes & PKG_CHANGED_STATE) != 0
- && fCoordinator == NULL) {
+ && fCoordinator.Get() == NULL) {
fWorkStatusView->PackageStatusChanged(ref);
}
}
@@ -1334,14 +1334,14 @@
{
AutoLocker<BLocker> 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<ProcessCoordinator>(item);
fCoordinator->Start();
}
else {
@@ -1364,7 +1364,7 @@
debugger("unable to release the process coordinator
sem");
{
AutoLocker<BLocker> lock(&fCoordinatorLock);
- if (fCoordinator == NULL)
+ if (fCoordinator.Get() == NULL)
return;
}
}
@@ -1381,16 +1381,15 @@
AutoLocker<BLocker> lock(&fCoordinatorLock);
while (!fCoordinatorQueue.empty()) {
- ProcessCoordinator *processCoordinator =
fCoordinatorQueue.front();
+ BReference<ProcessCoordinator> 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 @@
{
AutoLocker<BLocker> 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 @@
messenger.SendMessage(message);
}
- delete fCoordinator;
- fCoordinator = NULL;
+ fCoordinator = BReference<ProcessCoordinator>(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 32a3717..5ead8e5 100644
--- a/src/apps/haikudepot/ui/MainWindow.h
+++ b/src/apps/haikudepot/ui/MainWindow.h
@@ -155,9 +155,10 @@
Model fModel;
ModelListenerRef fModelListener;
- std::queue<ProcessCoordinator*>
+ std::queue<BReference<ProcessCoordinator>>
fCoordinatorQueue;
- ProcessCoordinator* fCoordinator;
+ BReference<ProcessCoordinator>
+ fCoordinator;
BLocker fCoordinatorLock;
sem_id fCoordinatorRunningSem;
--
To view, visit https://review.haiku-os.org/c/haiku/+/2797
To unsubscribe, or for help writing mail filters, visit
https://review.haiku-os.org/settings
Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: If535c151819da37d502283af3745e4148da69026
Gerrit-Change-Number: 2797
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Lindesay <apl@xxxxxxxxxxxxxx>
Gerrit-MessageType: newchange
