package daemon: Handle location info request in app thread

* ... instead of queuing it for the job thread. The advantage is that
the request will be handled immediately and clients won't have to wait
for transactions (which may even require user feedback) to finish. It
complicates Volume a bit, since there are now two threads that may
access it. The shared data have been moved to a State object which is
protected by a lock.
* For commit transaction requests check whether another package request
is already pending/in progress before queuing a job. Fail immediately,
if there is.

Fixes bug #10039.

headers/private/package/DaemonDefs.h

@@ -18,6 +18,7 @@ namespace BPrivate {
 
 enum BDaemonError {
 	B_DAEMON_OK = 0,
+	B_DAEMON_INSTALLATION_LOCATION_BUSY,
 	B_DAEMON_CHANGE_COUNT_MISMATCH,
 	B_DAEMON_BAD_REQUEST,
 	B_DAEMON_NO_SUCH_PACKAGE,

src/kits/package/DaemonClient.cpp

@@ -320,6 +320,9 @@ BDaemonClient::BCommitTransactionResult::FullErrorMessage() const
 	const char* errorString;
 	if (fError > 0) {
 		switch ((BDaemonError)fError) {
+			case B_DAEMON_INSTALLATION_LOCATION_BUSY:
+				errorString = "another package operation already in progress";
+				break;
 			case B_DAEMON_CHANGE_COUNT_MISMATCH:
 				errorString = "transaction out of date";
 				break;
@@ -339,7 +342,7 @@ BDaemonClient::BCommitTransactionResult::FullErrorMessage() const
 		}
 	} else
 		errorString = strerror(fError);
-		
+
 	BString result;
 	if (!fErrorMessage.IsEmpty()) {
 		result = fErrorMessage;

src/servers/package/JobQueue.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013, Haiku, Inc. All Rights Reserved.
+ * Copyright 2013-2014, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -12,6 +12,9 @@
 #include <PthreadMutexLocker.h>
 
 
+// #pragma mark - JobQueue
+
+
 JobQueue::JobQueue()
 	:
 	fMutexInitialized(false),
@@ -97,3 +100,25 @@ JobQueue::DequeueJob()
 
 	return NULL;
 }
+
+
+void
+JobQueue::DeleteJobs(Filter* filter)
+{
+	PthreadMutexLocker mutexLocker(fMutex);
+
+	for (JobList::Iterator it = fJobs.GetIterator(); Job* job = it.Next();) {
+		if (filter->FilterJob(job)) {
+			it.Remove();
+			delete job;
+		}
+	}
+}
+
+
+// #pragma mark - Filter
+
+
+JobQueue::Filter::~Filter()
+{
+}

src/servers/package/JobQueue.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013, Haiku, Inc. All Rights Reserved.
+ * Copyright 2013-2014, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -15,6 +15,9 @@
 
 
 class JobQueue {
+public:
+			class Filter;
+
 public:
 								JobQueue();
 								~JobQueue();
@@ -27,6 +30,8 @@ public:
 			Job*				DequeueJob();
 									// returns a reference
 
+			void				DeleteJobs(Filter* filter);
+
 private:
 			typedef DoublyLinkedList<Job> JobList;
 
@@ -40,4 +45,12 @@ private:
 };
 
 
+class JobQueue::Filter {
+public:
+	virtual						~Filter();
+
+	virtual	bool				FilterJob(Job* job) = 0;
+};
+
+
 #endif	// JOB_QUEUE_H

src/servers/package/PackageManager.cpp

@@ -132,7 +132,7 @@ PackageManager::InitInstalledRepository(InstalledRepository& repository)
 
 	if (Volume* volume = fRoot->GetVolume(repository.Location())) {
 		for (PackageFileNameHashTable::Iterator it
-				= volume->PackagesByFileName().GetIterator(); it.HasNext();) {
+				= volume->PackagesByFileNameIterator(); it.HasNext();) {
 			Package* package = it.Next();
 			if (package->IsActive()) {
 				BSolverPackage* solverPackage;

src/servers/package/Root.cpp

@@ -30,13 +30,36 @@ using namespace BPackageKit::BPrivate;
 using namespace BPackageKit::BManager::BPrivate;
 
 
+static const bigtime_t kCommunicationTimeout = 1000000;
+
+
+// #pragma mark - AbstractVolumeJob
+
+
+struct Root::AbstractVolumeJob : public Job {
+	AbstractVolumeJob(Volume* volume)
+		:
+		fVolume(volume)
+	{
+	}
+
+	Volume* GetVolume() const
+	{
+		return fVolume;
+	}
+
+protected:
+	Volume*	fVolume;
+};
+
+
 // #pragma mark - VolumeJob
 
 
-struct Root::VolumeJob : public Job {
+struct Root::VolumeJob : public AbstractVolumeJob {
 	VolumeJob(Volume* volume, void (Root::*method)(Volume*))
 		:
-		fVolume(volume),
+		AbstractVolumeJob(volume),
 		fMethod(method)
 	{
 	}
@@ -47,25 +70,49 @@ struct Root::VolumeJob : public Job {
 	}
 
 private:
-	Volume*	fVolume;
 	void	(Root::*fMethod)(Volume*);
 };
 
 
-// #pragma mark - RequestJob
+// #pragma mark - ProcessNodeMonitorEventsJob
 
 
-struct Root::RequestJob : public Job {
-	RequestJob(Root* root, BMessage* message)
+struct Root::ProcessNodeMonitorEventsJob : public VolumeJob {
+	ProcessNodeMonitorEventsJob(Volume* volume, void (Root::*method)(Volume*))
 		:
+		VolumeJob(volume, method)
+	{
+		fVolume->PackageJobPending();
+	}
+
+	~ProcessNodeMonitorEventsJob()
+	{
+		fVolume->PackageJobFinished();
+	}
+};
+
+
+// #pragma mark - CommitTransactionJob
+
+
+struct Root::CommitTransactionJob : public AbstractVolumeJob {
+	CommitTransactionJob(Root* root, Volume* volume, BMessage* message)
+		:
+		AbstractVolumeJob(volume),
 		fRoot(root),
 		fMessage(message)
 	{
+		fVolume->PackageJobPending();
+	}
+
+	~CommitTransactionJob()
+	{
+		fVolume->PackageJobFinished();
 	}
 
 	virtual void Do()
 	{
-		fRoot->_HandleRequest(fMessage.Get());
+		fRoot->_CommitTransaction(fVolume, fMessage.Get());
 	}
 
 private:
@@ -74,6 +121,27 @@ private:
 };
 
 
+// #pragma mark - VolumeJobFilter
+
+
+struct Root::VolumeJobFilter : public ::JobQueue::Filter {
+	VolumeJobFilter(Volume* volume)
+		:
+		fVolume(volume)
+	{
+	}
+
+	virtual bool FilterJob(Job* job)
+	{
+		AbstractVolumeJob* volumeJob = dynamic_cast<AbstractVolumeJob*>(job);
+		return volumeJob != NULL && volumeJob->GetVolume() == fVolume;
+	}
+
+private:
+	Volume*	fVolume;
+};
+
+
 // #pragma mark - Root
 
 
@@ -236,21 +304,64 @@ Root::GetVolume(BPackageInstallationLocation location)
 void
 Root::HandleRequest(BMessage* message)
 {
-	RequestJob* job = new(std::nothrow) RequestJob(this, message);
-	if (job == NULL) {
-		delete message;
+	ObjectDeleter<BMessage> messageDeleter(message);
+
+	// get the location and the volume
+	int32 location;
+	if (message->FindInt32("location", &location) != B_OK
+		|| location < 0
+		|| location >= B_PACKAGE_INSTALLATION_LOCATION_ENUM_COUNT) {
 		return;
 	}
 
-	_QueueJob(job);
+	AutoLocker<BLocker> locker(fLock);
+
+	Volume* volume = GetVolume((BPackageInstallationLocation)location);
+	if (volume == NULL)
+		return;
+
+	switch (message->what) {
+		case B_MESSAGE_GET_INSTALLATION_LOCATION_INFO:
+			volume->HandleGetLocationInfoRequest(message);
+			break;
+
+		case B_MESSAGE_COMMIT_TRANSACTION:
+		{
+			// The B_MESSAGE_COMMIT_TRANSACTION request must be handled in the
+			// job thread. But only queue a job, if there aren't package jobs
+			// pending already.
+			if (volume->IsPackageJobPending()) {
+				BMessage reply(B_MESSAGE_COMMIT_TRANSACTION_REPLY);
+				if (reply.AddInt32("error", B_DAEMON_INSTALLATION_LOCATION_BUSY)
+						== B_OK) {
+					message->SendReply(&reply, (BHandler*)NULL,
+						kCommunicationTimeout);
+				}
+				return;
+			}
+
+			CommitTransactionJob* job = new(std::nothrow) CommitTransactionJob(
+				this, volume, message);
+			if (job == NULL)
+				return;
+
+			messageDeleter.Detach();
+
+			_QueueJob(job);
+			break;
+		}
+
+		default:
+			break;
+	}
 }
 
 
 void
 Root::VolumeNodeMonitorEventOccurred(Volume* volume)
 {
-	_QueueJob(
-		new(std::nothrow) VolumeJob(volume, &Root::_ProcessNodeMonitorEvents));
+	_QueueJob(new(std::nothrow) ProcessNodeMonitorEventsJob(volume,
+		&Root::_ProcessNodeMonitorEvents));
 }
 
 
@@ -318,6 +429,10 @@ Root::_InitPackages(Volume* volume)
 void
 Root::_DeleteVolume(Volume* volume)
 {
+	// delete all pending jobs for that volume
+	VolumeJobFilter filter(volume);
+	fJobQueue.DeleteJobs(&filter);
+
 	delete volume;
 }
 
@@ -364,30 +479,9 @@ Root::_ProcessNodeMonitorEvents(Volume* volume)
 
 
 void
-Root::_HandleRequest(BMessage* message)
+Root::_CommitTransaction(Volume* volume, BMessage* message)
 {
-	int32 location;
-	if (message->FindInt32("location", &location) != B_OK
-		|| location < 0
-		|| location >= B_PACKAGE_INSTALLATION_LOCATION_ENUM_COUNT) {
-		return;
-	}
-
-	// get the volume and let it handle the message
-	AutoLocker<BLocker> locker(fLock);
-	Volume* volume = GetVolume((BPackageInstallationLocation)location);
-	locker.Unlock();
-
-	if (volume != NULL) {
-		switch (message->what) {
-			case B_MESSAGE_GET_INSTALLATION_LOCATION_INFO:
-				volume->HandleGetLocationInfoRequest(message);
-				break;
-			case B_MESSAGE_COMMIT_TRANSACTION:
-				volume->HandleCommitTransactionRequest(message);
-				break;
-		}
-	}
+	volume->HandleCommitTransactionRequest(message);
 }
 
 

src/servers/package/Root.h

@@ -55,10 +55,13 @@ protected:
 	virtual	void				LastReferenceReleased();
 
 private:
+			struct AbstractVolumeJob;
 			struct VolumeJob;
-			struct RequestJob;
+			struct ProcessNodeMonitorEventsJob;
+			struct CommitTransactionJob;
+			struct VolumeJobFilter;
 
-			friend struct RequestJob;
+			friend struct CommitTransactionJob;
 
 private:
 			Volume**			_GetVolume(PackageFSMountType mountType);
@@ -67,7 +70,8 @@ private:
 			void				_InitPackages(Volume* volume);
 			void				_DeleteVolume(Volume* volume);
 			void				_ProcessNodeMonitorEvents(Volume* volume);
-			void				_HandleRequest(BMessage* message);
+			void				_CommitTransaction(Volume* volume,
+									BMessage* message);
 
 			status_t			_QueueJob(Job* job);
 

src/servers/package/Volume.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013, Haiku, Inc. All Rights Reserved.
+ * Copyright 2013-2014, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -105,6 +105,146 @@ private:
 };
 
 
+// #pragma mark - State
+
+
+struct Volume::State {
+
+	State()
+		:
+		fLock("volume state"),
+		fPackagesByFileName(),
+		fPackagesByNodeRef(),
+		fChangeCount(0),
+		fPendingPackageJobCount(0)
+	{
+	}
+
+	~State()
+	{
+		fPackagesByFileName.Clear();
+
+		Package* package = fPackagesByNodeRef.Clear(true);
+		while (package != NULL) {
+			Package* next = package->NodeRefHashTableNext();
+			delete package;
+			package = next;
+		}
+	}
+
+	bool Init()
+	{
+		return fLock.InitCheck() == B_OK && fPackagesByFileName.Init() == B_OK
+			&& fPackagesByNodeRef.Init() == B_OK;
+	}
+
+	bool Lock()
+	{
+		return fLock.Lock();
+	}
+
+	void Unlock()
+	{
+		fLock.Unlock();
+	}
+
+	int64 ChangeCount() const
+	{
+		return fChangeCount;
+	}
+
+	Package* FindPackage(const char* name) const
+	{
+		return fPackagesByFileName.Lookup(name);
+	}
+
+	Package* FindPackage(const node_ref& nodeRef) const
+	{
+		return fPackagesByNodeRef.Lookup(nodeRef);
+	}
+
+	PackageFileNameHashTable::Iterator ByFileNameIterator() const
+	{
+		return fPackagesByFileName.GetIterator();
+	}
+
+	PackageNodeRefHashTable::Iterator ByNodeRefIterator() const
+	{
+		return fPackagesByNodeRef.GetIterator();
+	}
+
+	void AddPackage(Package* package)
+	{
+		AutoLocker<BLocker> locker(fLock);
+		fPackagesByFileName.Insert(package);
+		fPackagesByNodeRef.Insert(package);
+	}
+
+	void RemovePackage(Package* package)
+	{
+		AutoLocker<BLocker> locker(fLock);
+		_RemovePackage(package);
+	}
+
+	void SetPackageActive(Package* package, bool active)
+	{
+		AutoLocker<BLocker> locker(fLock);
+		package->SetActive(active);
+	}
+
+	void ActivationChanged(const PackageSet& activatedPackage,
+		const PackageSet& deactivatePackages)
+	{
+		AutoLocker<BLocker> locker(fLock);
+
+		for (PackageSet::iterator it = activatedPackage.begin();
+				it != activatedPackage.end(); ++it) {
+			(*it)->SetActive(true);
+			fChangeCount++;
+		}
+
+		for (PackageSet::iterator it = deactivatePackages.begin();
+				it != deactivatePackages.end(); ++it) {
+			Package* package = *it;
+			_RemovePackage(package);
+			delete package;
+		}
+	}
+
+	void PackageJobPending()
+	{
+		atomic_add(&fPendingPackageJobCount, 1);
+	}
+
+
+	void PackageJobFinished()
+	{
+		atomic_add(&fPendingPackageJobCount, -1);
+	}
+
+
+	bool IsPackageJobPending() const
+	{
+		return fPendingPackageJobCount != 0;
+	}
+
+private:
+	void _RemovePackage(Package* package)
+	{
+		fPackagesByFileName.Remove(package);
+		fPackagesByNodeRef.Remove(package);
+		fChangeCount++;
+	}
+
+private:
+	BLocker						fLock;
+	PackageFileNameHashTable	fPackagesByFileName;
+	PackageNodeRefHashTable		fPackagesByNodeRef;
+	int64						fChangeCount;
+	int32						fPendingPackageJobCount;
+};
+
+
 // #pragma mark - CommitTransactionHandler
 
 
@@ -159,7 +299,7 @@ struct Volume::CommitTransactionHandler {
 		BMessage* reply)
 	{
 		// check the change count
-		if (transaction.ChangeCount() != fVolume->fChangeCount)
+		if (transaction.ChangeCount() != fVolume->fState->ChangeCount())
 			throw Exception(B_DAEMON_CHANGE_COUNT_MISMATCH);
 
 		// collect the packages to deactivate
@@ -230,7 +370,7 @@ private:
 
 		for (int32 i = 0; i < packagesToDeactivateCount; i++) {
 			BString packageName = packagesToDeactivate.StringAt(i);
-			Package* package = fVolume->fPackagesByFileName.Lookup(packageName);
+			Package* package = fVolume->fState->FindPackage(packageName);
 			if (package == NULL) {
 				throw Exception(B_DAEMON_NO_SUCH_PACKAGE, "no such package",
 					packageName);
@@ -285,7 +425,7 @@ private:
 			BString packageName = packagesToActivate.StringAt(i);
 
 			// make sure it doesn't clash with an already existing package
-			Package* package = fVolume->fPackagesByFileName.Lookup(packageName);
+			Package* package = fVolume->fState->FindPackage(packageName);
 			if (package != NULL) {
 				if (fPackagesAlreadyAdded.find(package)
 						!= fPackagesAlreadyAdded.end()) {
@@ -1341,14 +1481,12 @@ Volume::Volume(BLooper* looper)
 	fPackagesDirectoryRef(),
 	fRoot(NULL),
 	fListener(NULL),
-	fPackagesByFileName(),
-	fPackagesByNodeRef(),
+	fState(NULL),
 	fPendingNodeMonitorEventsLock("pending node monitor events"),
 	fPendingNodeMonitorEvents(),
 	fNodeMonitorEventHandleTime(0),
 	fPackagesToBeActivated(),
 	fPackagesToBeDeactivated(),
-	fChangeCount(0),
 	fLocationInfoReply(B_MESSAGE_GET_INSTALLATION_LOCATION_INFO_REPLY)
 {
 	looper->AddHandler(this);
@@ -1360,21 +1498,15 @@ Volume::~Volume()
 	Unmounted();
 		// need for error case in InitPackages()
 
-	fPackagesByFileName.Clear();
-
-	Package* package = fPackagesByNodeRef.Clear(true);
-	while (package != NULL) {
-		Package* next = package->NodeRefHashTableNext();
-		delete package;
-		package = next;
-	}
+	delete fState;
 }
 
 
 status_t
 Volume::Init(const node_ref& rootDirectoryRef, node_ref& _packageRootRef)
 {
-	if (fPackagesByFileName.Init() != B_OK || fPackagesByNodeRef.Init() != B_OK)
+	fState = new(std::nothrow) State;
+	if (fState == NULL || !fState->Init())
 		RETURN_ERROR(B_NO_MEMORY);
 
 	fRootDirectoryRef = rootDirectoryRef;
@@ -1481,8 +1613,8 @@ Volume::InitPackages(Listener* listener)
 status_t
 Volume::AddPackagesToRepository(BSolverRepository& repository, bool activeOnly)
 {
-	for (PackageFileNameHashTable::Iterator it
-			= fPackagesByFileName.GetIterator(); it.HasNext();) {
+	for (PackageFileNameHashTable::Iterator it = fState->ByFileNameIterator();
+			it.HasNext();) {
 		Package* package = it.Next();
 		if (activeOnly && !package->IsActive())
 			continue;
@@ -1589,10 +1721,13 @@ INFORM("Volume::InitialVerify(%p, %p)\n", nextVolume, nextNextVolume);
 void
 Volume::HandleGetLocationInfoRequest(BMessage* message)
 {
+	AutoLocker<State> stateLocker(fState);
+
 	// If the cached reply message is up-to-date, just send it.
 	int64 changeCount;
 	if (fLocationInfoReply.FindInt64("change count", &changeCount) == B_OK
-		&& changeCount == fChangeCount) {
+		&& changeCount == fState->ChangeCount()) {
+		stateLocker.Unlock();
 		message->SendReply(&fLocationInfoReply, (BHandler*)NULL,
 			kCommunicationTimeout);
 		return;
@@ -1612,8 +1747,8 @@ Volume::HandleGetLocationInfoRequest(BMessage* message)
 		return;
 	}
 
-	for (PackageFileNameHashTable::Iterator it
-			= fPackagesByFileName.GetIterator(); it.HasNext();) {
+	for (PackageFileNameHashTable::Iterator it = fState->ByFileNameIterator();
+			it.HasNext();) {
 		Package* package = it.Next();
 		const char* fieldName = package->IsActive()
 			? "active packages" : "inactive packages";
@@ -1625,8 +1760,12 @@ Volume::HandleGetLocationInfoRequest(BMessage* message)
 		}
 	}
 
-	if (fLocationInfoReply.AddInt64("change count", fChangeCount) != B_OK)
+	if (fLocationInfoReply.AddInt64("change count", fState->ChangeCount())
+			!= B_OK) {
 		return;
+	}
+
+	stateLocker.Unlock();
 
 	message->SendReply(&fLocationInfoReply, (BHandler*)NULL,
 		kCommunicationTimeout);
@@ -1670,6 +1809,27 @@ Volume::HandleCommitTransactionRequest(BMessage* message)
 }
 
 
+void
+Volume::PackageJobPending()
+{
+	fState->PackageJobPending();
+}
+
+
+void
+Volume::PackageJobFinished()
+{
+	fState->PackageJobFinished();
+}
+
+
+bool
+Volume::IsPackageJobPending() const
+{
+	return fState->IsPackageJobPending();
+}
+
+
 void
 Volume::Unmounted()
 {
@@ -1738,6 +1898,13 @@ Volume::Location() const
 }
 
 
+PackageFileNameHashTable::Iterator
+Volume::PackagesByFileNameIterator() const
+{
+	return fState->ByFileNameIterator();
+}
+
+
 int
 Volume::OpenRootDirectory() const
 {
@@ -1852,7 +2019,7 @@ Volume::CreateTransaction(BPackageInstallationLocation location,
 	}
 
 	// init the transaction
-	error = _transaction.SetTo(location, fChangeCount, directoryName);
+	error = _transaction.SetTo(location, fState->ChangeCount(), directoryName);
 	if (error != B_OK) {
 		BEntry entry;
 		_transactionDirectory.GetEntry(&entry);
@@ -1979,7 +2146,7 @@ Volume::_PackagesEntryCreated(const char* name)
 {
 INFORM("Volume::_PackagesEntryCreated(\"%s\")\n", name);
 	// Ignore the event, if the package is already known.
-	Package* package = fPackagesByFileName.Lookup(name);
+	Package* package = fState->FindPackage(name);
 	if (package != NULL) {
 		if (package->EntryCreatedIgnoreLevel() > 0) {
 			package->DecrementEntryCreatedIgnoreLevel();
@@ -2029,7 +2196,7 @@ void
 Volume::_PackagesEntryRemoved(const char* name)
 {
 INFORM("Volume::_PackagesEntryRemoved(\"%s\")\n", name);
-	Package* package = fPackagesByFileName.Lookup(name);
+	Package* package = fState->FindPackage(name);
 	if (package == NULL)
 		return;
 
@@ -2081,16 +2248,13 @@ Volume::_FillInActivationChangeItem(PackageFSActivationChangeItem* item,
 void
 Volume::_AddPackage(Package* package)
 {
-	fPackagesByFileName.Insert(package);
-	fPackagesByNodeRef.Insert(package);
+	fState->AddPackage(package);
 }
 
 void
 Volume::_RemovePackage(Package* package)
 {
-	fPackagesByFileName.Remove(package);
-	fPackagesByNodeRef.Remove(package);
-	fChangeCount++;
+	fState->RemovePackage(package);
 }
 
 
@@ -2157,7 +2321,7 @@ Volume::_GetActivePackages(int fd)
 
 	// mark the returned packages active
 	for (uint32 i = 0; i < request->packageCount; i++) {
-		Package* package = fPackagesByNodeRef.Lookup(
+		Package* package = fState->FindPackage(
 			node_ref(request->infos[i].packageDeviceID,
 				request->infos[i].packageNodeID));
 		if (package == NULL) {
@@ -2169,18 +2333,17 @@ Volume::_GetActivePackages(int fd)
 			continue;
 		}
 
-		package->SetActive(true);
+		fState->SetPackageActive(package, true);
 INFORM("active package: \"%s\"\n", package->FileName().String());
 	}
 
-for (PackageNodeRefHashTable::Iterator it = fPackagesByNodeRef.GetIterator();
+for (PackageNodeRefHashTable::Iterator it = fState->ByNodeRefIterator();
 	it.HasNext();) {
 	Package* package = it.Next();
 	if (!package->IsActive())
 		INFORM("inactive package: \"%s\"\n", package->FileName().String());
 }
 
-			PackageNodeRefHashTable fPackagesByNodeRef;
 // INFORM("%" B_PRIu32 " active packages:\n", request->packageCount);
 // for (uint32 i = 0; i < request->packageCount; i++) {
 // 	INFORM("  dev: %" B_PRIdDEV ", node: %" B_PRIdINO "\n",
@@ -2280,8 +2443,8 @@ Volume::_CreateActivationFileContent(const PackageSet& toActivate,
 	const PackageSet& toDeactivate, BString& _content)
 {
 	BString activationFileContent;
-	for (PackageFileNameHashTable::Iterator it
-			= fPackagesByFileName.GetIterator(); it.HasNext();) {
+	for (PackageFileNameHashTable::Iterator it = fState->ByFileNameIterator();
+			it.HasNext();) {
 		Package* package = it.Next();
 		if (package->IsActive()
 			&& toDeactivate.find(package) == toDeactivate.end()) {
@@ -2446,16 +2609,5 @@ packagesToActivate.size(), packagesToDeactivate.size());
 
 	// Update our state, i.e. remove deactivated packages and mark activated
 	// packages accordingly.
-	for (PackageSet::iterator it = packagesToActivate.begin();
-		it != packagesToActivate.end(); ++it) {
-		(*it)->SetActive(true);
-		fChangeCount++;
-	}
-
-	for (PackageSet::iterator it = packagesToDeactivate.begin();
-		it != packagesToDeactivate.end(); ++it) {
-		Package* package = *it;
-		_RemovePackage(package);
-		delete package;
-	}
+	fState->ActivationChanged(packagesToActivate, packagesToDeactivate);
 }

src/servers/package/Volume.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013, Haiku, Inc. All Rights Reserved.
+ * Copyright 2013-2014, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -23,6 +23,21 @@
 #include "Package.h"
 
 
+// Locking Policy
+// ==============
+//
+// A Volume object is accessed by two threads:
+// 1. The application thread: initially (c'tor and Init()) and when handling a
+//    location info request (HandleGetLocationInfoRequest()).
+// 2. The corresponding Root object's job thread (any other operation).
+//
+// The only thread synchronization needed is for the status information accessed
+// by HandleGetLocationInfoRequest() and modified by the job thread. The data
+// are encapsulated in a Volume::State object which contains a lock. The lock
+// must be held by the app thread when accessing the data (it reads only) and
+// by the job thread when modifying the data (not needed when reading).
+
+
 using BPackageKit::BPrivate::BActivationTransaction;
 using BPackageKit::BPrivate::BDaemonClient;
 
@@ -61,6 +76,10 @@ public:
 			void				HandleCommitTransactionRequest(
 									BMessage* message);
 
+			void				PackageJobPending();
+			void				PackageJobFinished();
+			bool				IsPackageJobPending() const;
+
 			void				Unmounted();
 
 	virtual	void				MessageReceived(BMessage* message);
@@ -90,10 +109,8 @@ public:
 			void				SetRoot(Root* root)
 									{ fRoot = root; }
 
-			const PackageFileNameHashTable& PackagesByFileName() const
-									{ return fPackagesByFileName; }
-			const PackageNodeRefHashTable& PackagesByNodeRef() const
-									{ return fPackagesByNodeRef; }
+			PackageFileNameHashTable::Iterator PackagesByFileNameIterator()
+									const;
 
 			int					OpenRootDirectory() const;
 
@@ -120,6 +137,7 @@ public:
 
 private:
 			struct NodeMonitorEvent;
+			struct State;
 			struct CommitTransactionHandler;
 
 			friend struct CommitTransactionHandler;
@@ -196,15 +214,14 @@ private:
 			node_ref			fPackagesDirectoryRef;
 			Root*				fRoot;
 			Listener*			fListener;
-			PackageFileNameHashTable fPackagesByFileName;
-			PackageNodeRefHashTable fPackagesByNodeRef;
+			State*				fState;
 			BLocker				fPendingNodeMonitorEventsLock;
 			NodeMonitorEventList fPendingNodeMonitorEvents;
 			bigtime_t			fNodeMonitorEventHandleTime;
 			PackageSet			fPackagesToBeActivated;
 			PackageSet			fPackagesToBeDeactivated;
-			int64				fChangeCount;
 			BMessage			fLocationInfoReply;
+									// only accessed in the application thread
 };