BRoster::Launch() eventually launches the application in question

in several steps:
1. early pre-registration with the registrar ("I wanna launch the
   app, make sure noone interferes.")
2. load the app image
3. finish pre-registration with the registrar ("I have launched
   the app, here is its team ID.")
4. start app main thread
5. send "on launch" messages to the app (argv, refs, others)

If the app is already running or being launched, 1. fails with a
conclusive error code and returns the team ID and the pre-registration
token of the app. Steps 2 - 4 are skipped and only the messages are
delivered using the team ID returned by 1.

This change fixes a race condition: The failed early pre-registration
request obviously cannot return the team ID, if the other thread
launching the app has not finished step 3 yet. Thus the argv/refs
message would not get delivered and Launch() would not return the
correct team ID.

Now we wait for the pre-registration to be finished in this case, using
the former _IsAppPreRegistered() mechanism, which already provided
such a waiting feature for one request. It has been extended to
accomodate an arbitrary number of waiting requests and renamed to
_IsAppRegistered().

This fixed bug #763.



git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@18728 a95241bf-73f2-0310-859d-f6bbb57e9c96

docs/develop/servers/registrar/Protocols

@@ -155,23 +155,29 @@ error reply fields:
 
 -----------------------------------------------------------------------
 
-app pre-registration check (BRoster::IsAppPreRegistered())
+app registration check (BRoster::IsAppRegistered())
 
 target:		roster
-message:	B_REG_IS_APP_PRE_REGISTERED
-			"ref":	B_REF_TYPE
-			"team":	B_INT32_TYPE
+message:	B_REG_IS_APP_REGISTERED
+			"ref":		B_REF_TYPE
+			( "team":	B_INT32_TYPE
+			| "token":	B_INT32_TYPE )
 reply:		standard success
+			"registered":		B_BOOL_TYPE
 			"pre-registered":	B_BOOL_TYPE
+			[ "app_info":		flattened app_info ]
 on error:	- B_NO_REPLY (fatal)
 			- standard error
 
 message fields:
 - "ref": An entry_ref to the application executable.
 - "team": The application team (team_id).
+- "token": The application's preregistration token.
 
 reply fields:
+- "registered": true, if the app is (pre-)registered, false otherwise.
 - "pre-registered": true, if the app is pre-registered, false otherwise.
+- "app_info": Flattened app info, if the app is known (i.e. (pre-)registered).
 
 error reply fields:
 - "error":

headers/os/app/Roster.h

@@ -136,6 +136,8 @@ class BRoster {
 					port_id port) const;
 		bool _IsAppPreRegistered(const entry_ref *ref, team_id team,
 					app_info *info) const;
+		status_t _IsAppRegistered(const entry_ref *ref, team_id team,
+					uint32 token, bool *preRegistered, app_info *info) const;
 		status_t _RemovePreRegApp(uint32 entryToken) const;
 		status_t _RemoveApp(team_id team) const;
 		void _ApplicationCrashed(team_id team);

headers/private/app/RegistrarDefs.h

@@ -55,7 +55,7 @@ enum {
 	// roster requests
 	B_REG_ADD_APP							= 'rgaa',
 	B_REG_COMPLETE_REGISTRATION				= 'rgcr',
-	B_REG_IS_APP_PRE_REGISTERED				= 'rgip',
+	B_REG_IS_APP_REGISTERED					= 'rgip',
 	B_REG_REMOVE_PRE_REGISTERED_APP			= 'rgrp',
 	B_REG_REMOVE_APP						= 'rgra',
 	B_REG_SET_THREAD_AND_TEAM				= 'rgtt',

headers/private/app/RosterPrivate.h

@@ -43,9 +43,10 @@ class BRoster::Private {
 					port_id port) const
 			{ return fRoster->_CompleteRegistration(team, thread, port); }
 
-		bool IsAppPreRegistered(const entry_ref *ref, team_id team,
-					app_info *info) const
-			{ return fRoster->_IsAppPreRegistered(ref, team, info); }
+		status_t IsAppRegistered(const entry_ref *ref, team_id team,
+					uint32 token, bool *preRegistered, app_info *info) const
+			{ return fRoster->_IsAppRegistered(ref, team, token, preRegistered,
+					info); }
 
 		status_t RemoveApp(team_id team) const
 			{ return fRoster->_RemoveApp(team); }

src/kits/app/Application.cpp

@@ -318,8 +318,10 @@ BApplication::_InitData(const char *signature, bool initGUI, status_t *_error)
 	bool preRegistered = false;
 	app_info appInfo;
 	if (fInitError == B_OK && !isRegistrar) {
-		preRegistered = BRoster::Private().IsAppPreRegistered(&ref, team,
-							&appInfo);
+		if (BRoster::Private().IsAppRegistered(&ref, team, 0, &preRegistered,
+				&appInfo) != B_OK) {
+			preRegistered = false;
+		}
 	}
 	if (preRegistered) {
 		// we are pre-registered => the app info has been filled in

src/kits/app/Roster.cpp

@@ -1424,10 +1424,12 @@ BRoster::_AddApplication(const char *mimeSig, const entry_ref *ref,
 		error = request.AddInt32("port", port);
 	if (error == B_OK)
 		error = request.AddBool("full_registration", fullReg);
+
 	// send the request
 	BMessage reply;
 	if (error == B_OK)
 		error = fMessenger.SendMessage(&request, &reply);
+
 	// evaluate the reply
 	if (error == B_OK) {
 		if (reply.what == B_REG_SUCCESS) {
@@ -1442,8 +1444,11 @@ BRoster::_AddApplication(const char *mimeSig, const entry_ref *ref,
 		} else {
 			if (reply.FindInt32("error", &error) != B_OK)
 				error = B_ERROR;
+			// get team and token from the reply
 			if (otherTeam && reply.FindInt32("other_team", otherTeam) != B_OK)
 				*otherTeam = -1;
+			if (pToken && reply.FindInt32("token", (int32*)pToken) != B_OK)
+				*pToken = 0;
 		}
 	}
 	return error;
@@ -1580,46 +1585,66 @@ BRoster::_CompleteRegistration(team_id team, thread_id thread,
 	return error;
 }
 
-// IsAppPreRegistered
-/*!	\brief Returns whether an application is pre-registered.
+// _IsAppRegistered
+/*!	\brief Returns whether an application is registered.
 
 	If the application is indeed pre-registered and \a info is not \c NULL,
 	the methods fills in the app_info structure pointed to by \a info.
 
 	\param ref An entry_ref referring to the app's executable
-	\param team The app's team ID
+	\param team The app's team ID. May be -1, if \a token is given.
+	\param token The app's pre-registration token. May be 0, if \a team is
+				 given.
+	\param preRegistered: Pointer to a pre-allocated bool to be filled in
+		   by this method, indicating whether or not the app was
+		   pre-registered.
 	\param info A pointer to a pre-allocated app_info structure to be filled
 		   in by this method (may be \c NULL)
-	\return \c true, if the application is pre-registered, \c false if not.
+	\return
+		- \c B_OK, if the application is registered and all requested
+		  information could be retrieved,
+		- another error code, if the app is not registered or an error occurred.
 */
-bool
-BRoster::_IsAppPreRegistered(const entry_ref *ref, team_id team,
-	app_info *info) const
+status_t
+BRoster::_IsAppRegistered(const entry_ref *ref, team_id team,
+	uint32 token, bool *preRegistered, app_info *info) const
 {
 	status_t error = B_OK;
+
 	// compose the request message
-	BMessage request(B_REG_IS_APP_PRE_REGISTERED);
+	BMessage request(B_REG_IS_APP_REGISTERED);
 	if (error == B_OK && ref)
 		error = request.AddRef("ref", ref);
 	if (error == B_OK && team >= 0)
 		error = request.AddInt32("team", team);
+	if (error == B_OK && token > 0)
+		error = request.AddInt32("token", (int32)token);
+	
 	// send the request
 	BMessage reply;
 	if (error == B_OK)
 		error = fMessenger.SendMessage(&request, &reply);
 
 	// evaluate the reply
+	bool isRegistered = false;
 	bool isPreRegistered = false;
 	if (error == B_OK) {
 		if (reply.what == B_REG_SUCCESS) {
-			if (reply.FindBool("pre-registered", &isPreRegistered) != B_OK)
+			if (reply.FindBool("registered", &isRegistered) != B_OK
+				|| !isRegistered
+				|| reply.FindBool("pre-registered", &isPreRegistered) != B_OK) {
 				error = B_ERROR;
-			if (error == B_OK && isPreRegistered && info)
+			}
+
+			if (error == B_OK && preRegistered)
+				*preRegistered = isPreRegistered;
+			if (error == B_OK && info)
 				error = find_message_app_info(&reply, info);
 		} else if (reply.FindInt32("error", &error) != B_OK)
 			error = B_ERROR;
 	}
-	return error == B_OK && isPreRegistered;
+	
+	return error;
 }
 
 // RemovePreRegApp
@@ -1815,10 +1840,13 @@ BRoster::_LaunchApp(const char *mimeType, const entry_ref *ref,
 		if (error == B_ALREADY_RUNNING) {
 			DBG(OUT("  already running\n"));
 			alreadyRunning = true;
-			error = B_OK;
+
 			// get the app flags for the running application
-			if (GetRunningAppInfo(team, &appInfo) == B_OK)
+			error = _IsAppRegistered(&appRef, team, appToken, NULL, &appInfo);
+			if (error == B_OK) {
 				otherAppFlags = appInfo.flags;
+				team = appInfo.team;
+			}
 		}
 		DBG(OUT("  pre-register: %s (%lx)\n", strerror(error), error));
 	}

src/servers/registrar/Registrar.cpp

@@ -126,8 +126,8 @@ Registrar::MessageReceived(BMessage *message)
 		case B_REG_COMPLETE_REGISTRATION:
 			fRoster->HandleCompleteRegistration(message);
 			break;
-		case B_REG_IS_APP_PRE_REGISTERED:
-			fRoster->HandleIsAppPreRegistered(message);
+		case B_REG_IS_APP_REGISTERED:
+			fRoster->HandleIsAppRegistered(message);
 			break;
 		case B_REG_REMOVE_PRE_REGISTERED_APP:
 			fRoster->HandleRemovePreRegApp(message);

src/servers/registrar/TRoster.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2001-2005, Ingo Weinhold, bonefish@users.sf.net.
+ * Copyright 2001-2006, Ingo Weinhold, bonefish@users.sf.net.
  * Distributed under the terms of the MIT License.
  */
 
@@ -65,9 +65,8 @@ static bool larger_index(const recent_entry *entry1, const recent_entry *entry2)
 	are one or more instances of the application that are pre-registered, but
 	have no team ID assigned yet, the reply to the request has to be
 	postponed until the status of the requesting team is clear. The request
-	message is dequeued from the registrar's message queue and, with
-	additional information (IAPRRequest), added to \a fIAPRRequests for a
-	later reply.
+	message is dequeued from the registrar's message queue and added to
+	\a fIARRequestsByID for a later reply.
 
 	The field \a fActiveApp identifies the currently active application
 	and \a fLastToken is a counter used to generate unique tokens for
@@ -115,7 +114,8 @@ TRoster::TRoster()
 	   : fLock("roster"),
 	     fRegisteredApps(),
 		 fEarlyPreRegisteredApps(),
-		 fIAPRRequests(),
+		 fIARRequestsByID(),
+		 fIARRequestsByToken(),
 		 fActiveApp(NULL),
 		 fWatchingService(),
 		 fRecentApps(),
@@ -176,6 +176,8 @@ PRINT(("full registration: %d\n", fullReg));
 
 	// check the parameters
 	team_id otherTeam = -1;
+	uint32 token = 0;
+	
 	uint32 launchFlags = flags & B_LAUNCH_MASK;
 
 	// entry_ref
@@ -206,6 +208,7 @@ PRINT(("ref: %ld, %lld, %s\n", ref.device, ref.directory, ref.name));
 				|| ((info = fEarlyPreRegisteredApps.InfoFor(signature))))) {
 			SET_ERROR(error, B_ALREADY_RUNNING);
 			otherTeam = info->team;
+			token = info->token;
 		}
 	}
 
@@ -219,7 +222,6 @@ PRINT(("ref: %ld, %lld, %s\n", ref.device, ref.directory, ref.name));
 	}
 
 	// Add the application info.
-	uint32 token = 0;
 	if (error == B_OK) {
 		// alloc and init the info
 		RosterAppInfo *info = new(nothrow) RosterAppInfo;
@@ -270,6 +272,8 @@ PRINT(("added to early pre-regs, token: %lu\n", token));
 		reply.AddInt32("error", error);
 		if (otherTeam >= 0)
 			reply.AddInt32("other_team", otherTeam);
+		if (token > 0)
+			reply.AddInt32("token", (int32)token);
 		request->SendReply(&reply);
 	}
 
@@ -339,12 +343,12 @@ TRoster::HandleCompleteRegistration(BMessage *request)
 	FUNCTION_END();
 }
 
-// HandleIsAppPreRegistered
-/*!	\brief Handles an IsAppPreRegistered() request.
+// HandleIsAppRegistered
+/*!	\brief Handles an IsAppRegistered() request.
 	\param request The request message
 */
 void
-TRoster::HandleIsAppPreRegistered(BMessage *request)
+TRoster::HandleIsAppRegistered(BMessage *request)
 {
 	FUNCTION_START();
 
@@ -354,36 +358,48 @@ TRoster::HandleIsAppPreRegistered(BMessage *request)
 	// get the parameters
 	entry_ref ref;
 	team_id team;
+	uint32 token;
 	if (request->FindRef("ref", &ref) != B_OK)
 		SET_ERROR(error, B_BAD_VALUE);
 	if (request->FindInt32("team", &team) != B_OK)
 		team = -1;
-PRINT(("team: %ld\n", team));
+	if (request->FindInt32("token", (int32*)&token) != B_OK)
+		token = 0;
+PRINT(("team: %ld, token: %lu\n", team, token));
 PRINT(("ref: %ld, %lld, %s\n", ref.device, ref.directory, ref.name));
+
 	// check the parameters
 	// entry_ref
 	if (error == B_OK & !BEntry(&ref).Exists())
 		SET_ERROR(error, B_ENTRY_NOT_FOUND);
-	// team
-	if (error == B_OK && team < 0)
+	// team/token
+	if (error == B_OK && team < 0 && token == 0)
 		SET_ERROR(error, B_BAD_VALUE);
+
 	// look up the information
 	RosterAppInfo *info = NULL;
 	if (error == B_OK) {
 		if ((info = fRegisteredApps.InfoFor(team)) != NULL) {
 PRINT(("found team in fRegisteredApps\n"));
-			_ReplyToIAPRRequest(request, info);
-		} else if ((info = fEarlyPreRegisteredApps.InfoFor(&ref)) != NULL) {
-PRINT(("found ref in fEarlyRegisteredApps\n"));
+			_ReplyToIARRequest(request, info);
+		} else if (token > 0
+			&& (info = fEarlyPreRegisteredApps.InfoForToken(token)) != NULL) {
+PRINT(("found ref in fEarlyRegisteredApps (by token)\n"));
 			// pre-registered and has no team ID assigned yet -- queue the
 			// request
 			be_app->DetachCurrentMessage();
-			IAPRRequest queuedRequest = { ref, team, request };
-			fIAPRRequests[team] = queuedRequest;
+			_AddIARRequest(fIARRequestsByToken, team, request);
+		} else if (team >= 0
+			&& (info = fEarlyPreRegisteredApps.InfoFor(&ref)) != NULL) {
+PRINT(("found ref in fEarlyRegisteredApps (by ref)\n"));
+			// pre-registered and has no team ID assigned yet -- queue the
+			// request
+			be_app->DetachCurrentMessage();
+			_AddIARRequest(fIARRequestsByID, team, request);
 		} else {
 PRINT(("didn't find team or ref\n"));
-			// team not registered, ref not early pre-registered
-			_ReplyToIAPRRequest(request, NULL);
+			// team not registered, ref/token not early pre-registered
+			_ReplyToIARRequest(request, NULL);
 		}
 	} else {
 		// reply to the request on error
@@ -524,14 +540,23 @@ PRINT(("team: %ld, thread: %ld, token: %lu\n", team, thread, token));
 					delete_port(info->port);
 				delete info;
 			}
-			// handle a pending IsAppPreRegistered() request
-			IAPRRequestMap::iterator it = fIAPRRequests.find(team);
-			if (it != fIAPRRequests.end()) {
-				IAPRRequest &request = it->second;
+			// handle pending IsAppRegistered() requests
+			IARRequestMap::iterator it = fIARRequestsByID.find(team);
+			if (it != fIARRequestsByID.end()) {
+				BMessageQueue *requests = it->second;
 				if (error == B_OK)
-					_ReplyToIAPRRequest(request.request, info);
-				delete request.request;
-				fIAPRRequests.erase(it);
+					_ReplyToIARRequests(requests, info);
+				delete requests;
+				fIARRequestsByID.erase(it);
+			}
+
+			it = fIARRequestsByToken.find((int32)token);
+			if (it != fIARRequestsByToken.end()) {
+				BMessageQueue *requests = it->second;
+				if (error == B_OK)
+					_ReplyToIARRequests(requests, info);
+				delete requests;
+				fIARRequestsByToken.erase(it);
 			}
 		} else
 			SET_ERROR(error, B_REG_APP_NOT_PRE_REGISTERED);
@@ -1543,11 +1568,57 @@ TRoster::_NextToken()
 	return ++fLastToken;
 }
 
-// _ReplyToIAPRRequest
-/*!	\brief Sends a reply message to a IsAppPreRegistered() request.
+// _AddIARRequest
+/*!	\brief Adds an IsAppRegistered() request to the given map.
+
+	If something goes wrong, the method deletes the request.
+
+	\param map The map the request shall be added to.
+	\param key The key under which to add the request.
+	\param request The request message to be added.
+*/
+void
+TRoster::_AddIARRequest(IARRequestMap& map, int32 key, BMessage* request)
+{
+	IARRequestMap::iterator it = map.find(key);
+	BMessageQueue* requests = NULL;
+	if (it == map.end()) {
+		requests = new(nothrow) BMessageQueue();
+		if (!requests) {
+			delete request;
+			return;
+		}
+
+		map[key] = requests;
+	} else
+		requests = it->second;
+
+	requests->AddMessage(request);
+}
+
+// _ReplyToIARRequests
+/*!	\brief Invokes _ReplyToIARRequest() for all messages in the given
+		   message queue.
+
+	\param requests The request messages to be replied to
+	\param info The RosterAppInfo of the application in question
+		   (may be \c NULL)
+*/
+void
+TRoster::_ReplyToIARRequests(BMessageQueue *requests,
+	const RosterAppInfo *info)
+{
+	while (BMessage* request = requests->NextMessage()) {
+		_ReplyToIARRequest(request, info);
+		delete request;
+	}
+}
+
+// _ReplyToIARRequest
+/*!	\brief Sends a reply message to an IsAppRegistered() request.
 
 	The message to be sent is a simple \c B_REG_SUCCESS message containing
-	a "pre-registered" field, that sais whether or not the application is
+	a "pre-registered" field, that says whether or not the application is
 	pre-registered. It will be set to \c false, unless an \a info is supplied
 	and the application this info refers to is pre-registered.
 
@@ -1556,7 +1627,7 @@ TRoster::_NextToken()
 		   (may be \c NULL)
 */
 void
-TRoster::_ReplyToIAPRRequest(BMessage *request, const RosterAppInfo *info)
+TRoster::_ReplyToIARRequest(BMessage *request, const RosterAppInfo *info)
 {
 	// pre-registered or registered?
 	bool preRegistered = false;
@@ -1573,9 +1644,10 @@ TRoster::_ReplyToIAPRRequest(BMessage *request, const RosterAppInfo *info)
 	}
 	// send reply
 	BMessage reply(B_REG_SUCCESS);
+	reply.AddBool("registered", (bool)info);
 	reply.AddBool("pre-registered", preRegistered);
-PRINT(("_ReplyToIAPRRequest(): pre-registered: %d\n", preRegistered));
-	if (preRegistered)
+PRINT(("_ReplyToIARRequest(): pre-registered: %d\n", preRegistered));
+	if (info)
 		_AddMessageAppInfo(&reply, info);
 	request->SendReply(&reply);
 }

src/servers/registrar/TRoster.h

@@ -32,6 +32,7 @@
 #include <map>
 
 #include <Locker.h>
+#include <MessageQueue.h>
 #include <SupportDefs.h>
 
 #include "AppInfoList.h"
@@ -48,13 +49,7 @@ using std::map;
 class BMessage;
 class WatchingService;
 
-struct IAPRRequest {
-	entry_ref	ref;
-	team_id		team;
-	BMessage	*request;
-};
-
-typedef map<team_id, IAPRRequest>	IAPRRequestMap;
+typedef map<int32, BMessageQueue*>	IARRequestMap;
 
 class TRoster {
 public:
@@ -63,7 +58,7 @@ public:
 
 	void HandleAddApplication(BMessage *request);
 	void HandleCompleteRegistration(BMessage *request);
-	void HandleIsAppPreRegistered(BMessage *request);
+	void HandleIsAppRegistered(BMessage *request);
 	void HandleRemovePreRegApp(BMessage *request);
 	void HandleRemoveApp(BMessage *request);
 	void HandleSetThreadAndTeam(BMessage *request);
@@ -115,7 +110,11 @@ private:
 	static status_t _AddMessageWatchingInfo(BMessage *message,
 											const app_info *info);
 	uint32 _NextToken();
-	void _ReplyToIAPRRequest(BMessage *request, const RosterAppInfo *info);
+
+	void _AddIARRequest(IARRequestMap& map, int32 key, BMessage* request);
+	void _ReplyToIARRequests(BMessageQueue *requests,
+		const RosterAppInfo *info);
+	void _ReplyToIARRequest(BMessage *request, const RosterAppInfo *info);
 
 	void _HandleGetRecentEntries(BMessage *request);
 
@@ -129,7 +128,8 @@ private:
 	BLocker			fLock;
 	AppInfoList		fRegisteredApps;
 	AppInfoList		fEarlyPreRegisteredApps;
-	IAPRRequestMap	fIAPRRequests;
+	IARRequestMap	fIARRequestsByID;
+	IARRequestMap	fIARRequestsByToken;
 	RosterAppInfo	*fActiveApp;
 	WatchingService	fWatchingService;
 	RecentApps		fRecentApps;