From ab306fb8f5e1fe4c3fdc97ef61a497c9e0e7c78f Mon Sep 17 00:00:00 2001
From: Michael Lotz <mmlr@mlotz.ch>
Date: Mon, 9 Nov 2015 22:42:51 +0100
Subject: [PATCH] BPathMonitor: Fix locking order reversal introduced in
 8599f4b.

The sLocker was used as an outer lock with the sLooper locked within.
The sLocker therefore can't be used within MessageReceived() as that
could lead to deadlocks due to reversal of the locking order.

Instead of two locks, just use locking the looper for all serialization.
While this has a higher overhead to using a BLocker (due to the looper
list locking and lookups) this shouldn't be too problematic.
---
 src/kits/storage/PathMonitor.cpp | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/src/kits/storage/PathMonitor.cpp b/src/kits/storage/PathMonitor.cpp
index bb6db839ef..2462437313 100644
--- a/src/kits/storage/PathMonitor.cpp
+++ b/src/kits/storage/PathMonitor.cpp
@@ -58,7 +58,6 @@ typedef BOpenHashTable<WatcherHashDefinition> WatcherMap;
 
 static pthread_once_t sInitOnce = PTHREAD_ONCE_INIT;
 static WatcherMap* sWatchers = NULL;
-static BLocker* sLocker = NULL;
 static BLooper* sLooper = NULL;
 static BPathMonitor::BWatchingInterface* sDefaultWatchingInterface = NULL;
 static BPathMonitor::BWatchingInterface* sWatchingInterface = NULL;
@@ -840,10 +839,7 @@ PathHandler::PathHandler(const char* path, uint32 flags,
 		return;
 
 	// add ourselves to the looper
-	if (!looper->Lock())
-		debugger("PathHandler: failed to lock the looper");
 	looper->AddHandler(this);
-	looper->Unlock();
 
 	// start watching
 	fStatus = _StartWatchingAncestors(fRoot, false);
@@ -878,13 +874,9 @@ PathHandler::InitCheck() const
 void
 PathHandler::Quit()
 {
-	if (sLooper->Lock()) {
-		TRACE("%p->PathHandler::Quit()\n", this);
-		sWatchingInterface->StopWatching(this);
-		sLooper->RemoveHandler(this);
-		sLooper->Unlock();
-	} else
-		TRACE("%p->PathHandler::Quit(): failed to lock looper\n", this);
+	TRACE("%p->PathHandler::Quit()\n", this);
+	sWatchingInterface->StopWatching(this);
+	sLooper->RemoveHandler(this);
 	delete this;
 }
 
@@ -899,7 +891,6 @@ PathHandler::MessageReceived(BMessage* message)
 			if (message->FindInt32("opcode", &opcode) != B_OK)
 				return;
 
-			BAutolock _(sLocker);
 			switch (opcode) {
 				case B_ENTRY_CREATED:
 					_EntryCreated(message);
@@ -2001,7 +1992,7 @@ BPathMonitor::StartWatching(const char* path, uint32 flags,
 	if (status != B_OK)
 		return status;
 
-	BAutolock _(sLocker);
+	BAutolock _(sLooper);
 
 	Watcher* watcher = sWatchers->Lookup(target);
 	bool newWatcher = false;
@@ -2051,12 +2042,12 @@ BPathMonitor::StartWatching(const char* path, uint32 flags,
 /*static*/ status_t
 BPathMonitor::StopWatching(const char* path, const BMessenger& target)
 {
-	if (sLocker == NULL)
+	if (sLooper == NULL)
 		return B_BAD_VALUE;
 
 	TRACE("BPathMonitor::StopWatching(%s)\n", path);
 
-	BAutolock _(sLocker);
+	BAutolock _(sLooper);
 
 	Watcher* watcher = sWatchers->Lookup(target);
 	if (watcher == NULL)
@@ -2081,10 +2072,10 @@ BPathMonitor::StopWatching(const char* path, const BMessenger& target)
 /*static*/ status_t
 BPathMonitor::StopWatching(const BMessenger& target)
 {
-	if (sLocker == NULL)
+	if (sLooper == NULL)
 		return B_BAD_VALUE;
 
-	BAutolock _(sLocker);
+	BAutolock _(sLooper);
 
 	Watcher* watcher = sWatchers->Lookup(target);
 	if (watcher == NULL)
@@ -2124,11 +2115,6 @@ BPathMonitor::_InitIfNeeded()
 /*static*/ void
 BPathMonitor::_Init()
 {
-	sLocker = new (std::nothrow) BLocker("path monitor");
-	TRACE("Create PathMonitor locker\n");
-	if (sLocker == NULL)
-		return;
-
 	sDefaultWatchingInterface = new(std::nothrow) BWatchingInterface;
 	if (sDefaultWatchingInterface == NULL)
 		return;