app_server doesn't hang anymore when an application exits in an unclean way. Got rid of the kill_thread in ServerApp's destructor. Small refactoring. Added a TODO item.

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

src/servers/app/AppServer.cpp

@@ -608,15 +608,6 @@ void AppServer::DispatchMessage(int32 code, BPortLink &msg)
 					srvapp=(ServerApp *)fAppList->RemoveItem(i);
 					if(srvapp)
 					{
-						status_t		temp;
-						// TODO: This call never returns, thus screwing the
-						// app server completely: it's easy to test:
-						// run any test app which creates a window, quit
-						// the application clicking on the window's "close" button,
-						// and try to launch the application again. It won't start.
-						// Anyway, this should be moved to ~ServerApp() (which has already
-						// a "kill_thread()" call, btw).
-						wait_for_thread(srvapp_id, &temp);
 						delete srvapp;
 						srvapp= NULL;
 					}

src/servers/app/ServerApp.cpp

@@ -85,6 +85,8 @@
 */
 ServerApp::ServerApp(port_id sendport, port_id rcvport, port_id clientLooperPort,
 		team_id clientTeamID, int32 handlerID, char *signature)
+		:
+		fQuitting(false)
 {
 	// it will be of *very* musch use in correct window order
 	fClientTeamID		= clientTeamID;
@@ -140,33 +142,23 @@ ServerApp::ServerApp(port_id sendport, port_id rcvport, port_id clientLooperPort
 ServerApp::~ServerApp(void)
 {
 	STRACE(("*ServerApp %s:~ServerApp()\n",fSignature.String()));
-	int32 i;
 	
-	ServerBitmap *tempbmp;
+	fQuitting = true;
-	for(i=0;i<fBitmapList->CountItems();i++)
+	
-	{
+	for (int32 i = 0; i< fBitmapList->CountItems(); i++)
-		tempbmp=(ServerBitmap*)fBitmapList->ItemAt(i);
+		delete static_cast<ServerBitmap *>(fBitmapList->ItemAt(i));
-		if(tempbmp)
+	
-			delete tempbmp;
-	}
 	fBitmapList->MakeEmpty();
 	delete fBitmapList;
 
-	ServerPicture *temppic;
+	for (int32 i = 0; i < fPictureList->CountItems(); i++)
-	for(i=0;i<fPictureList->CountItems();i++)
+		delete static_cast<ServerPicture *>(fPictureList->ItemAt(i));
-	{
+	
-		temppic=(ServerPicture*)fPictureList->ItemAt(i);
-		if(temppic)
-			delete temppic;
-	}
 	fPictureList->MakeEmpty();
 	delete fPictureList;
 
 	delete fMsgReader;
-	fMsgReader=NULL;
-	
 	delete fMsgSender;
-	fMsgSender=NULL;
 
 	// This shouldn't be necessary -- all cursors owned by the app
 	// should be cleaned up by RemoveAppCursors	
@@ -181,12 +173,21 @@ ServerApp::~ServerApp(void)
 	
 	STRACE(("#ServerApp %s:~ServerApp()\n",fSignature.String()));
 
+	// TODO: Is this the right place for this ?
+	// From what I've understood, this is the port created by
+	// the BApplication (?), but if I delete it in there, GetNextMessage()
+	// in the MonitorApp thread never returns. Cleanup.
+	delete_port(fMessagePort);
+	
 	// Kill the monitor thread if it exists
 	thread_info info;
-	if(get_thread_info(fMonitorThreadID,&info)==B_OK)
+	status_t dummyStatus;
-		kill_thread(fMonitorThreadID);
+	if (get_thread_info(fMonitorThreadID, &info) == B_OK)
+		wait_for_thread(fMonitorThreadID, &dummyStatus);
 	
 	delete fSharedMem;
+	
+	STRACE(("ServerApp %s::~ServerApp(): Exiting\n", fSignature.String()));
 }
 
 /*!
@@ -304,17 +305,18 @@ int32 ServerApp::MonitorApp(void *data)
 	ServerApp *app = (ServerApp *)data;
 	LinkMsgReader msgqueue(app->fMessagePort);
 	
-	bool quitting = false;
 	int32 code;
 	status_t err = B_OK;
 	
-	while(!quitting)
+	while(!app->fQuitting)
 	{
 		STRACE(("info: ServerApp::MonitorApp listening on port %ld.\n", app->fMessagePort));
 //		err = msgqueue.GetNextReply(&code);
 		err = msgqueue.GetNextMessage(&code);
-		if (err < B_OK)
+		if (err < B_OK) {
+			STRACE(("ServerApp::MonitorApp(): GetNextMessage returned %s\n", strerror(err)));
 			break;
+		}
 
 		switch(code)
 		{
@@ -390,13 +392,10 @@ int32 ServerApp::MonitorApp(void *data)
 			{
 				STRACE(("ServerApp %s: B_QUIT_REQUESTED\n",app->fSignature.String()));
 				// Our BApplication sent us this message when it quit.
-				// We need to ask the app_server to delete our monitor
+				// We need to ask the app_server to delete ourself.
-				// ADI: No! This is a bad solution. A thead should continue its
+				app->fQuitting = true;
-				// execution until its exit point, and this can *very* easily be done
+				
-				quitting=true;
+				port_id	serverport = find_port(SERVER_PORT_NAME);
-				// see... no need to ask the main thread to kill us.
-				// still... it will delete this ServerApp object.
-				port_id		serverport = find_port(SERVER_PORT_NAME);
 				if(serverport == B_NAME_NOT_FOUND){
 					printf("PANIC: ServerApp %s could not find the app_server port!\n",app->fSignature.String());
 					break;

src/servers/app/ServerApp.h

@@ -79,7 +79,7 @@ public:
 	
 	FMWList fAppFMWList;
 const char * Title() const { return fSignature.String(); }
-protected:
+private:
 	friend class AppServer;
 	friend class ServerWindow;
 	
@@ -108,6 +108,8 @@ protected:
 	bool fIsActive;
 	int32 fHandlerToken;
 	AreaPool *fSharedMem;
+	
+	bool fQuitting;
 };
 
 #endif