launch_daemon: Delegate launch data replies to Job.

Previously the LaunchDaemon would send out its own team id when a given
job was not yet launched, leading to invalid BMessengers once the port
owner changed to the actually launched team.

The launch of the target team and the launch data replies were also not
synchronized, which could lead to the launched team getting a reply
pointing to the launch_daemon when requesting data for itself. This is
the case for the BRoster init of the registrar. The fix in hrev49561
therefore didn't always work, because the registrar would sometimes get
the launch_daemon team id instead of the id of itself. It would later
try talking to the launch_daemon, which obviously never replied, leading
to #12237.

The LaunchDaemon now delegates the launch data reply to the Job instead.
The Job either replies directly, in case it has already been launched,
or queues the reply for when the launch completes. This causes launch
data requesters to block until the launch attempt is completed, but
won't block the LaunchDaemon message loop.

This commit introduces the seperate fLaunchStatus to properly handle the
ambiguity of fTeam being < 0, which is the case for both, when no launch
was attempted and when the launch failed. This new field now determines
what IsLaunched() returns and how launch data replies are handled.

The new launch status is additionally protected by the launch status
lock, which will later probably be made broader in scope to protect
against race conditions once service monitoring is implemented.

src/servers/launch/Job.cpp

@@ -27,8 +27,11 @@ Job::Job(const char* name)
 	fCreateDefaultPort(false),
 	fInitStatus(B_NO_INIT),
 	fTeam(-1),
-	fTarget(NULL)
+	fLaunchStatus(B_NO_INIT),
+	fTarget(NULL),
+	fPendingLaunchDataReplies(0, false)
 {
+	mutex_init(&fLaunchStatusLock, "launch status lock");
 }
 
 
@@ -40,8 +43,12 @@ Job::Job(const Job& other)
 	fCreateDefaultPort(other.CreateDefaultPort()),
 	fInitStatus(B_NO_INIT),
 	fTeam(-1),
-	fTarget(other.Target())
+	fLaunchStatus(B_NO_INIT),
+	fTarget(other.Target()),
+	fPendingLaunchDataReplies(0, false)
 {
+	mutex_init(&fLaunchStatusLock, "launch status lock");
+
 	fCondition = other.fCondition;
 	// TODO: copy events
 	//fEvent = other.fEvent;
@@ -343,16 +350,21 @@ Job::Launch()
 		// Launch by signature
 		BString signature("application/");
 		signature << Name();
-		return BRoster::Private().Launch(signature.String(), NULL, NULL,
-			0, NULL, &environment[0], &fTeam);
+
+		status_t status = BRoster::Private().Launch(signature.String(), NULL,
+			NULL, 0, NULL, &environment[0], &fTeam);
+		_SetLaunchStatus(status);
+		return status;
 	}
 
 	// Build argument vector
 
 	entry_ref ref;
 	status_t status = get_ref_for_path(fArguments.StringAt(0).String(), &ref);
-	if (status != B_OK)
+	if (status != B_OK) {
+		_SetLaunchStatus(status);
 		return status;
+	}
 
 	std::vector<const char*> args;
 
@@ -365,15 +377,28 @@ Job::Launch()
 	}
 
 	// Launch via entry_ref
-	return BRoster::Private().Launch(NULL, &ref, NULL, count, &args[0],
+	status = BRoster::Private().Launch(NULL, &ref, NULL, count, &args[0],
 		&environment[0], &fTeam);
+	_SetLaunchStatus(status);
+	return status;
 }
 
 
 bool
 Job::IsLaunched() const
 {
-	return fTeam >= 0;
+	return fLaunchStatus != B_NO_INIT;
+}
+
+
+status_t
+Job::HandleGetLaunchData(BMessage* message)
+{
+	MutexLocker launchLocker(fLaunchStatusLock);
+	if (IsLaunched())
+		return _SendLaunchDataReply(message);
+
+	return fPendingLaunchDataReplies.AddItem(message) ? B_OK : B_NO_MEMORY;
 }
 
 
@@ -434,3 +459,49 @@ Job::_AddStringList(std::vector<const char*>& array, const BStringList& list)
 		array.push_back(list.StringAt(index).String());
 	}
 }
+
+
+void
+Job::_SetLaunchStatus(status_t launchStatus)
+{
+	MutexLocker launchLocker(fLaunchStatusLock);
+	fLaunchStatus = launchStatus != B_NO_INIT ? launchStatus : B_ERROR;
+	launchLocker.Unlock();
+
+	_SendPendingLaunchDataReplies();
+}
+
+
+status_t
+Job::_SendLaunchDataReply(BMessage* message)
+{
+	BMessage reply(fTeam < 0 ? fTeam : (uint32)B_OK);
+	if (reply.what == B_OK) {
+		reply.AddInt32("team", fTeam);
+
+		PortMap::const_iterator iterator = fPortMap.begin();
+		for (; iterator != fPortMap.end(); iterator++) {
+			BString name;
+			if (iterator->second.HasString("name"))
+				name << iterator->second.GetString("name") << "_";
+			name << "port";
+
+			reply.AddInt32(name.String(),
+				iterator->second.GetInt32("port", -1));
+		}
+	}
+
+	message->SendReply(&reply);
+	delete message;
+	return B_OK;
+}
+
+
+void
+Job::_SendPendingLaunchDataReplies()
+{
+	for (int32 i = 0; i < fPendingLaunchDataReplies.CountItems(); i++)
+		_SendLaunchDataReply(fPendingLaunchDataReplies.ItemAt(i));
+
+	fPendingLaunchDataReplies.MakeEmpty();
+}

src/servers/launch/Job.h

@@ -15,6 +15,8 @@
 #include <OS.h>
 #include <StringList.h>
 
+#include <locks.h>
+
 
 using namespace BSupportKit;
 class BMessage;
@@ -68,6 +70,8 @@ public:
 			status_t			Launch();
 			bool				IsLaunched() const;
 
+			status_t			HandleGetLaunchData(BMessage* message);
+
 protected:
 	virtual	status_t			Execute();
 
@@ -78,6 +82,11 @@ private:
 			void				_AddStringList(std::vector<const char*>& array,
 									const BStringList& list);
 
+			void				_SetLaunchStatus(status_t launchStatus);
+
+			status_t			_SendLaunchDataReply(BMessage* message);
+			void				_SendPendingLaunchDataReplies();
+
 private:
 			BStringList			fArguments;
 			BStringList			fRequirements;
@@ -87,8 +96,12 @@ private:
 			PortMap				fPortMap;
 			status_t			fInitStatus;
 			team_id				fTeam;
+			status_t			fLaunchStatus;
+			mutex				fLaunchStatusLock;
 			::Target*			fTarget;
 			::Condition*		fCondition;
+			BObjectList<BMessage>
+								fPendingLaunchDataReplies;
 };
 
 

src/servers/launch/LaunchDaemon.cpp

@@ -483,31 +483,29 @@ LaunchDaemon::_HandleGetLaunchData(BMessage* message)
 				launchJob = false;
 			}
 		}
-	}
+	} else
+		launchJob = false;
 
+	bool ownsMessage = false;
 	if (reply.what == B_OK) {
-		// If the job has not been launched yet, we'll pass on our
-		// team here. The rationale behind this is that this team
-		// will temporarily own the synchronous reply ports.
-		reply.AddInt32("team", job->Team() < 0
-			? current_team() : job->Team());
-
-		PortMap::const_iterator iterator = job->Ports().begin();
-		for (; iterator != job->Ports().end(); iterator++) {
-			BString name;
-			if (iterator->second.HasString("name"))
-				name << iterator->second.GetString("name") << "_";
-			name << "port";
-
-			reply.AddInt32(name.String(),
-				iterator->second.GetInt32("port", -1));
-		}
-
 		// Launch the job if it hasn't been launched already
 		if (launchJob)
 			_LaunchJob(job);
+
+		DetachCurrentMessage();
+		status_t result = job->HandleGetLaunchData(message);
+		if (result == B_OK) {
+			// Replying is delegated to the job.
+			return;
 		}
+
+		ownsMessage = true;
+		reply.what = result;
+	}
+
 	message->SendReply(&reply);
+	if (ownsMessage)
+		delete message;
 }