BAppServerLink is now using BApplication::fServerTo/From for its messaging.

Added LinkMsgReader::NeedsReply() method.
Completely redone ServerApp messaging: no more "replyport" from BAppServerLink; instead,
the registered client reply port is used. Fixed some more weak messaging stuff.
ServerApp now recognizes if an unknown message needs a reply, and sends it - for example,
the "Screen" preferences app no longer hangs, but crashes on start :)
Made LinkMsgReader::Read() virtual again, since it's needed by RAMLinkMsgReader.cpp.
Renamed BPortLink::GetNextReply() to GetNextMessage().
Some more cleanup.


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

headers/private/app/LinkMsgReader.h

@@ -23,7 +23,9 @@ class LinkMsgReader {
 		port_id	Port(void) { return fReceivePort; }
 
 		status_t GetNextMessage(int32 &code, bigtime_t timeout = B_INFINITE_TIMEOUT);
-		status_t Read(void *data, ssize_t size);
+		bool NeedsReply() const;
+
+		virtual status_t Read(void *data, ssize_t size);
 		status_t ReadString(char **string);
 		template <class Type> status_t Read(Type *data)
 		{

headers/private/app/PortLink.h

@@ -61,7 +61,8 @@ class BPortLink {
 		void SetReplyPort(port_id port);
 		port_id	ReplyPort();
 
-		status_t GetNextReply(int32 &code, bigtime_t timeout = B_INFINITE_TIMEOUT);
+		status_t GetNextMessage(int32 &code, bigtime_t timeout = B_INFINITE_TIMEOUT);
+		bool NeedsReply() const;
 		status_t Read(void *data, ssize_t size);
 		status_t ReadString(char **string);
 		status_t ReadRegion(BRegion *region);
@@ -71,6 +72,8 @@ class BPortLink {
 		// convenience methods
 
 		status_t FlushWithReply(int32 &code);
+		LinkMsgReader &Reader() { return *fReader; }
+		LinkMsgSender &Sender() { return *fSender; }
 
 	protected:
 		LinkMsgReader *fReader;
@@ -148,11 +151,17 @@ BPortLink::ReplyPort()
 }
 
 inline status_t
-BPortLink::GetNextReply(int32 &code, bigtime_t timeout)
+BPortLink::GetNextMessage(int32 &code, bigtime_t timeout)
 {
 	return fReader->GetNextMessage(code, timeout);
 }
 
+inline bool
+BPortLink::NeedsReply() const
+{
+	return fReader->NeedsReply();
+}
+
 inline status_t
 BPortLink::Read(void *data, ssize_t size)
 {

src/kits/app/AppServerLink.cpp

@@ -34,7 +34,6 @@
 
 
 BLocker sLock;
-port_id sReplyPort = -1;
 
 
 namespace BPrivate {
@@ -44,15 +43,10 @@ BAppServerLink::BAppServerLink(void)
 	sLock.Lock();
 
 	// if there is no be_app, we can't do a whole lot, anyway
-	if (be_app)
+	if (be_app) {
 		SetSendPort(be_app->fServerFrom);
-
+		SetReplyPort(be_app->fServerTo);
-	// There is only one global reply port, and we create it here
+	}
-	// (protected by sLock) when it's not yet there
-	if (sReplyPort < B_OK)
-		sReplyPort = create_port(100, "AppServerLink reply port");
-
-	SetReplyPort(sReplyPort);
 }
 
 
@@ -65,17 +59,11 @@ BAppServerLink::~BAppServerLink()
 status_t
 BAppServerLink::FlushWithReply(int32 *code)
 {
-	status_t err;
+	status_t status = Flush(B_INFINITE_TIMEOUT, true);
+	if (status < B_OK)
+		return status;
 
-	err = Attach<port_id>(sReplyPort);
+	return GetNextMessage(*code);
-	if (err < B_OK)
-		return err;
-
-	err = Flush();
-	if (err < B_OK)
-		return err;
-
-	return GetNextReply(*code);
 }
 
 }	// namespace BPrivate

src/kits/app/LinkMsgReader.cpp

@@ -102,6 +102,17 @@ LinkMsgReader::GetNextMessage(int32 &code, bigtime_t timeout)
 }
 
 
+bool
+LinkMsgReader::NeedsReply() const
+{
+	if (fReplySize == 0)
+		return false;
+
+	message_header *header = (message_header *)(fRecvBuffer + fRecvStart);
+	return (header->flags & kNeedsReply) != 0;
+}
+
+
 void
 LinkMsgReader::ResetBuffer()
 {

src/kits/app/PortLink.cpp

@@ -92,9 +92,9 @@ BPortLink::AttachShape(BShape &shape)
 status_t
 BPortLink::FlushWithReply(int32 &code)
 {
-	status_t status = Flush();
+	status_t status = Flush(B_INFINITE_TIMEOUT, true);
 	if (status < B_OK)
 		return status;
 
-	return GetNextReply(code);
+	return GetNextMessage(code);
 }

src/servers/app/AppServer.cpp

@@ -257,22 +257,16 @@ AppServer::~AppServer(void)
 int32
 AppServer::PicassoThread(void *data)
 {
-	int32		i;
+	for (;;) {
-	AppServer	*appserver=(AppServer*)data;
+		acquire_sem(sAppServer->fAppListLock);
-	ServerApp	*app;
+		for (int32 i = 0;;) {
-	for(;;)
+			ServerApp *app = (ServerApp *)sAppServer->fAppList->ItemAt(i++);
-	{
+			if (!app)
-		i = 0;
-		acquire_sem(appserver->fAppListLock);
-		for(;;)
-		{
-			app=(ServerApp*)appserver->fAppList->ItemAt(i++);
-			if(!app)
 				break;
 
 			app->PingTarget();
 		}
-		release_sem(appserver->fAppListLock);		
+		release_sem(sAppServer->fAppListLock);		
 		// we do this every other second so as not to suck *too* many CPU cycles
 		snooze(1000000);
 	}
@@ -407,7 +401,7 @@ AppServer::MainLoop(void)
 		STRACE(("info: AppServer::MainLoop listening on port %ld.\n", fMessagePort));
 
 		int32 code;
-		status_t err = pmsg.GetNextReply(code);
+		status_t err = pmsg.GetNextMessage(code);
 		if (err < B_OK) {
 			STRACE(("MainLoop:pmsg.GetNextReply failed\n"));
 			continue;
@@ -424,12 +418,12 @@ AppServer::MainLoop(void)
 			case AS_SET_DECORATOR:
 			case AS_GET_DECORATOR:
 			case AS_R5_SET_DECORATOR:
-				DispatchMessage(code,pmsg);
+				DispatchMessage(code, pmsg);
 				break;
 			default:
 			{
-				STRACE(("Server::MainLoop received unexpected code %ld(offset %ld)\n",
+				STRACE(("Server::MainLoop received unexpected code %ld (offset %ld)\n",
-						code,code-SERVER_TRUE));
+						code, code - SERVER_TRUE));
 				break;
 			}
 		}
@@ -464,8 +458,8 @@ AppServer::LoadDecorator(const char *path)
 	status_t			stat;
 	image_id			addon;
 	
-	addon= load_add_on(path);
+	addon = load_add_on(path);
-	if(addon < 0)
+	if (addon < B_OK)
 		return false;
 
 	// As of now, we do nothing with decorator versions, but the possibility exists
@@ -474,9 +468,9 @@ AppServer::LoadDecorator(const char *path)
 	// go here.
 
 	// Get the instantiation function
-	stat= get_image_symbol(addon, "instantiate_decorator", B_SYMBOL_TYPE_TEXT, (void**)&pcreatefunc);
+	stat = get_image_symbol(addon, "instantiate_decorator",
-	if(stat != B_OK)
+		B_SYMBOL_TYPE_TEXT, (void**)&pcreatefunc);
-	{
+	if (stat != B_OK) {
 		unload_add_on(addon);
 		return false;
 	}
@@ -497,33 +491,29 @@ AppServer::InitDecorators(void)
 {
 	BMessage settings;
 
-	BDirectory dir,newdir;
+	BDirectory dir;
-	if(dir.SetTo(SERVER_SETTINGS_DIR)==B_ENTRY_NOT_FOUND)
+	if (dir.SetTo(SERVER_SETTINGS_DIR) == B_ENTRY_NOT_FOUND)
-		create_directory(SERVER_SETTINGS_DIR,0777);
+		create_directory(SERVER_SETTINGS_DIR, 0777);
 
 	BString path(SERVER_SETTINGS_DIR);
-	path+="DecoratorSettings";
+	path += "DecoratorSettings";
-	BFile file(path.String(),B_READ_ONLY);
+	BFile file(path.String(), B_READ_ONLY);
 
-	if(file.InitCheck()==B_OK)
+	if (file.InitCheck() == B_OK
-	{
+		&& settings.Unflatten(&file) == B_OK) {
-		if(settings.Unflatten(&file)==B_OK)
-		{
 		BString itemtext;
-			if(settings.FindString("decorator",&itemtext)==B_OK)
+		if (settings.FindString("decorator", &itemtext) == B_OK) {
-			{
 			path.SetTo(DECORATORS_DIR);
-				path+=itemtext;
+			path += itemtext;
-				if(LoadDecorator(path.String()))
+			if (LoadDecorator(path.String()))
 				return;
 		}
 	}
-	}
 
 	// We got this far, so something must have gone wrong. We set make_decorator
 	// to NULL so that the decorator allocation routine knows to utilize the included
 	// default decorator instead of an addon.
-	make_decorator=NULL;
+	make_decorator = NULL;
 }
 
 /*!
@@ -550,25 +540,26 @@ AppServer::DispatchMessage(int32 code, BPortLink &msg)
 			// Find the necessary data
 			team_id	clientTeamID = -1;
 			port_id	clientLooperPort = -1;
-			port_id app_port = -1;
+			port_id clientReplyPort = -1;
 			int32 htoken = B_NULL_TOKEN;
-			char *app_signature = NULL;
+			char *appSignature = NULL;
 
-			msg.Read<port_id>(&app_port);
+			msg.Read<port_id>(&clientReplyPort);
 			msg.Read<port_id>(&clientLooperPort);
 			msg.Read<team_id>(&clientTeamID);
 			msg.Read<int32>(&htoken);
-			msg.ReadString(&app_signature);
+			if (msg.ReadString(&appSignature) != B_OK)
+				break;
 
-			port_id server_listen = create_port(DEFAULT_MONITOR_PORT_SIZE, app_signature);
+			port_id serverListen = create_port(DEFAULT_MONITOR_PORT_SIZE, appSignature);
-			if (server_listen < B_OK) {
+			if (serverListen < B_OK) {
 				printf("No more ports left. Time to crash. Have a nice day! :)\n");
 				break;
 			}
 
 			// we let the application own the port, so that we get aware when it's gone
-			if (set_port_owner(server_listen, clientTeamID) < B_OK) {
+			if (set_port_owner(serverListen, clientTeamID) < B_OK) {
-				delete_port(server_listen);
+				delete_port(serverListen);
 				printf("Could not transfer port ownership to client %ld!\n", clientTeamID);
 				break;
 			}
@@ -576,21 +567,21 @@ AppServer::DispatchMessage(int32 code, BPortLink &msg)
 			// Create the ServerApp subthread for this app
 			acquire_sem(fAppListLock);
 
-			ServerApp *app = new ServerApp(app_port,server_listen, clientLooperPort,
+			ServerApp *app = new ServerApp(clientReplyPort, serverListen, clientLooperPort,
-				clientTeamID, htoken, app_signature);
+				clientTeamID, htoken, appSignature);
 
 			// add the new ServerApp to the known list of ServerApps
 			fAppList->AddItem(app);
 
 			release_sem(fAppListLock);
 
-			BPortLink replylink(app_port);
+			BPortLink replylink(clientReplyPort);
 			replylink.StartMessage(SERVER_TRUE);
-			replylink.Attach<int32>(server_listen);
+			replylink.Attach<int32>(serverListen);
 			replylink.Flush();
 
 			// This is necessary because BPortLink::ReadString allocates memory
-			free(app_signature);
+			free(appSignature);
 			break;
 		}
 		case AS_DELETE_APP:

src/servers/app/RAMLinkMsgReader.cpp

@@ -25,8 +25,8 @@ RAMLinkMsgReader::~RAMLinkMsgReader(void)
 }
 
 
-
+void
-void RAMLinkMsgReader::SetBuffer(int8 *buffer)
+RAMLinkMsgReader::SetBuffer(int8 *buffer)
 {
 	if(!buffer)
 	{
@@ -48,93 +48,35 @@ void RAMLinkMsgReader::SetBuffer(int8 *buffer)
 }
 
 
-int8 *RAMLinkMsgReader::GetBuffer(void)
+int8 *
+RAMLinkMsgReader::GetBuffer(void)
 {
 	return fBuffer;
 }
 
 
-size_t RAMLinkMsgReader::GetBufferSize(void)
+size_t
+RAMLinkMsgReader::GetBufferSize(void)
 {
 	return fAttachSize;
 }
 
 
-
+status_t
-status_t RAMLinkMsgReader::Read(void *data, ssize_t size)
+RAMLinkMsgReader::Read(void *data, ssize_t size)
 {
-	if(!fBuffer || fAttachSize==0)
+	if (!fBuffer || fAttachSize == 0)
 		return B_NO_INIT;
 
-	if(size<1)
+	if (size < 1)
 		return B_BAD_VALUE;
 
-	if(fPosition+size > fAttachStart+fAttachSize)
+	if (fPosition + size > fAttachStart + fAttachSize) {
-	{
 		// read past end of buffer
 		return B_BAD_VALUE;
 	}
 
 	memcpy(data, fPosition, size);
-	fPosition+=size;
+	fPosition += size;
 	return B_OK;
 }
-
-
-status_t RAMLinkMsgReader::ReadString(char **string)
-{
-	status_t err;
-	int32 len = 0;
-
-	err = Read<int32>(&len);
-	if (err < B_OK)
-		return err;
-
-	if (len)
-	{
-		*string = (char *)malloc(len);
-		if (*string == NULL)
-		{
-			fPosition -= sizeof(int32);
-			return B_NO_MEMORY;
-		}
-
-		err = Read(*string, len);
-		if (err < B_OK)
-		{
-			free(*string);
-			*string = NULL;
-			fPosition -= sizeof(int32);
-			return err;
-		}
-		(*string)[len-1] = '\0';
-		return B_OK;
-	}
-	else
-	{
-		fPosition -= sizeof(int32);
-		return B_ERROR;
-	}
-}
-
-// "Forbidden" functions :P
-status_t RAMLinkMsgReader::GetNextMessage(int32 *code, bigtime_t timeout)
-{
-	debugger("RAMLinkMsgReader::GetNextMessage is not permitted");
-	return B_ERROR;
-}
-
-
-void RAMLinkMsgReader::SetPort(port_id port)
-{
-	debugger("RAMLinkMsgReader::SetPort is not permitted");
-}
-
-
-port_id RAMLinkMsgReader::GetPort(void)
-{
-	debugger("RAMLinkMsgReader::GetPort is not permitted");
-	return B_ERROR;
-}
-
-

src/servers/app/RAMLinkMsgReader.h

@@ -39,34 +39,20 @@
 		size_t buffer size
 		[data buffer]
 */
-class RAMLinkMsgReader : public LinkMsgReader
+class RAMLinkMsgReader : public LinkMsgReader {
-{
+	public:
-public:
 		RAMLinkMsgReader(int8 *buffer);
 		RAMLinkMsgReader(void);
-	~RAMLinkMsgReader(void);
+		virtual ~RAMLinkMsgReader(void);
 
 		void SetBuffer(int8 *buffer);
 		int8 *GetBuffer(void);
 		size_t GetBufferSize(void);
 		int32 Code(void) { return fCode; }
 
-	status_t Read(void *data, ssize_t size);
+		virtual status_t Read(void *data, ssize_t size);
-	status_t ReadString(char **string);
-	template <class Type> status_t Read(Type *data)
-	{
-		return Read(data, sizeof(Type));
-	}
-
-	// These should never need to be called where this class is used. However, we do
-	// need to make debugging easier for such contexts...
-	status_t GetNextMessage(int32 *code, bigtime_t timeout);
-	void SetPort(port_id port);
-	port_id GetPort(void);
-	
-protected:
-	
 
+	protected:
 		int8 *fBuffer, *fAttachStart;
 		int8 *fPosition;
 		size_t fAttachSize;

src/servers/app/RootLayer.cpp

@@ -208,7 +208,7 @@ RootLayer::WorkingThread(void *data)
 	
 	STRACE(("info: RootLayer(%s)::WorkingThread listening on port %ld.\n", oneRootLayer->GetName(), oneRootLayer->fListenPort));
 	for (;;) {
-		err = messageQueue.GetNextReply(code);
+		err = messageQueue.GetNextMessage(code);
 		if (err < B_OK) {
 			STRACE(("WorkingThread: messageQueue.GetNextReply failed\n"));
 			continue;

src/servers/app/ServerApp.h

@@ -30,6 +30,7 @@
 
 #include <OS.h>
 #include <String.h>
+#include <PortLink.h>
 
 #include "FMWList.h"
 
@@ -38,8 +39,6 @@ class BMessage;
 class BPortLink;
 class BList;
 class DisplayDriver;
-class LinkMsgReader;
-class LinkMsgSender;
 class ServerPicture;
 class ServerCursor;
 class ServerBitmap;
@@ -110,8 +109,7 @@ private:
 	thread_id fMonitorThreadID;
 	team_id fClientTeamID;
 
-	LinkMsgReader *fMsgReader;
+	BPortLink fLink;
-	LinkMsgSender *fMsgSender;
 
 	// TODO:
 	// - Are really Bitmaps and Pictures stored per application and not globally ?

src/tests/kits/app/messaging/PortLinkTest.cpp

@@ -12,7 +12,7 @@ void
 get_next_message(BPortLink &link, int32 expectedCode)
 {
 	int32 code;
-	if (link.GetNextReply(code) != B_OK) {
+	if (link.GetNextMessage(code) != B_OK) {
 		fprintf(stderr, "get message failed!\n");
 		exit(-1);
 	}
@@ -96,7 +96,7 @@ main()
 	get_next_message(receiver, 'tst5');
 
 	int32 code;
-	status = receiver.GetNextReply(code, 0);
+	status = receiver.GetNextMessage(code, 0);
 	if (status != B_WOULD_BLOCK) {
 		fprintf(stderr, "reading would not block!\n");
 		return -1;