app_server & Interface Kit: Rework cursor reference management.

We can't allow applications to reference/unreference cursors,
this is a safety/security violation, and it being done improperly
lead to the reference counts becoming incorrect on the app_server
side.

Change AS_REFERENCE_CURSOR to AS_CLONE_CURSOR and adjust the Cursor
code appropriately. (In the future, copying BCursor without Clone'ing
the data in the case of custom cursors could be accomplished with
client-side reference counting.)

Then rework CursorManager to remove cursors at once on team deletion,
and otherwise clean up cursor reference management to let the
reference counting handle things.

headers/private/app/ServerProtocol.h

@@ -74,7 +74,7 @@ enum {
 
 	AS_CREATE_CURSOR,
 	AS_CREATE_CURSOR_BITMAP,
-	AS_REFERENCE_CURSOR,
+	AS_CLONE_CURSOR,
 	AS_DELETE_CURSOR,
 
 	AS_BEGIN_RECT_TRACKING,

src/kits/app/Cursor.cpp

@@ -10,11 +10,6 @@
 
 /**	BCursor describes a view-wide or application-wide cursor. */
 
-/**
-	@note:	As BeOS only supports 16x16 monochrome cursors, and I would like
-			to see a nice shadowes one, we will need to extend this one.
- */
-
 
 #include <AppDefs.h>
 #include <Bitmap.h>
@@ -160,14 +155,20 @@ BCursor::operator=(const BCursor& other)
 		_FreeCursorData();
 
 		fServerToken = other.fServerToken;
-		fNeedToFree = other.fNeedToFree;
 
-		if (fNeedToFree) {
-			// Tell app_server that there is another reference for this
-			// cursor data!
+		if (other.fNeedToFree) {
 			BPrivate::AppServerLink link;
-			link.StartMessage(AS_REFERENCE_CURSOR);
-			link.Attach<int32>(fServerToken);
+			link.StartMessage(AS_CLONE_CURSOR);
+			link.Attach<int32>(other.fServerToken);
+
+			status_t status;
+			if (link.FlushWithReply(status) == B_OK) {
+				if (status == B_OK) {
+					link.Read<int32>(&fServerToken);
+					fNeedToFree = true;
+				} else
+					fServerToken = status;
+			}
 		}
 	}
 	return *this;
@@ -212,4 +213,3 @@ BCursor::_FreeCursorData()
 		link.Flush();
 	}
 }
-

src/servers/app/CursorManager.cpp

@@ -182,28 +182,20 @@ CursorManager::AddCursor(ServerCursor* cursor, int32 token)
 }
 
 
-/*!	\brief Removes a cursor if it's not referenced anymore.
+/*!	\brief Removes a cursor.
 
 	If this was the last reference to this cursor, it will be deleted.
-	Only if the cursor is deleted, \c true is returned.
 */
-bool
+void
 CursorManager::RemoveCursor(ServerCursor* cursor)
 {
 	if (!Lock())
-		return false;
-
-	// TODO: this doesn't work as it looks like, and it's not safe!
-	if (cursor->CountReferences() > 0) {
-		// cursor has been referenced again in the mean time
-		Unlock();
-		return false;
-	}
+		return;
 
 	_RemoveCursor(cursor);
+	cursor->ReleaseReference();
 
 	Unlock();
-	return true;
 }
 
 
@@ -218,8 +210,11 @@ CursorManager::DeleteCursors(team_id team)
 
 	for (int32 index = fCursorList.CountItems(); index-- > 0;) {
 		ServerCursor* cursor = (ServerCursor*)fCursorList.ItemAtFast(index);
-		if (cursor->OwningTeam() == team)
-			cursor->ReleaseReference();
+		if (cursor->OwningTeam() != team)
+			continue;
+
+		_RemoveCursor(cursor);
+		cursor->ReleaseReference();
 	}
 
 	Unlock();
@@ -439,4 +434,5 @@ CursorManager::_RemoveCursor(ServerCursor* cursor)
 {
 	fCursorList.RemoveItem(cursor);
 	fTokenSpace.RemoveToken(cursor->fToken);
+	cursor->fToken = -1;
 }

src/servers/app/CursorManager.h

@@ -43,7 +43,7 @@ public:
 									int32 token = -1);
 			void				DeleteCursors(team_id team);
 
-			bool				RemoveCursor(ServerCursor* cursor);
+			void				RemoveCursor(ServerCursor* cursor);
 
 			void				SetCursorSet(const char* path);
 			ServerCursor*		GetCursor(BCursorID which);

src/servers/app/ProfileMessageSupport.cpp

@@ -55,7 +55,7 @@ string_for_message_code(uint32 code)
 
 		CODE(AS_CREATE_CURSOR);
 		CODE(AS_CREATE_CURSOR_BITMAP);
-		CODE(AS_REFERENCE_CURSOR);
+		CODE(AS_CLONE_CURSOR);
 		CODE(AS_DELETE_CURSOR);
 
 		CODE(AS_BEGIN_RECT_TRACKING);

src/servers/app/ServerApp.cpp

@@ -1248,27 +1248,46 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 			break;
 		}
 
-		case AS_REFERENCE_CURSOR:
+		case AS_CLONE_CURSOR:
 		{
 			STRACE(("ServerApp %s: Reference BCursor\n", Signature()));
 
 			// Attached data:
-			// 1) int32 token ID of the cursor to reference
+			// 1) int32 token ID of the cursor to clone
 
 			int32 token;
 			if (link.Read<int32>(&token) != B_OK)
 				break;
 
-			if (!fDesktop->GetCursorManager().Lock())
-				break;
+			status_t status = B_ERROR;
+			ServerCursor* cursor = NULL;
 
-			ServerCursor* cursor
-				= fDesktop->GetCursorManager().FindCursor(token);
-			if (cursor != NULL)
-				cursor->AcquireReference();
+			if (fDesktop->GetCursorManager().Lock()) {
+				ServerCursor* existingCursor
+					= fDesktop->GetCursorManager().FindCursor(token);
+				if (existingCursor != NULL)
+					cursor = new(std::nothrow) ServerCursor(existingCursor);
+				if (cursor != NULL) {
+					cursor->SetOwningTeam(fClientTeam);
+					token = fDesktop->GetCursorManager().AddCursor(cursor);
+					if (token < 0) {
+						delete cursor;
+						cursor = NULL;
+					}
+				}
 
-			fDesktop->GetCursorManager().Unlock();
+				fDesktop->GetCursorManager().Unlock();
+			}
 
+			if (cursor != NULL) {
+				// Synchronous message - BApplication is waiting on the
+				// cursor's ID
+				fLink.StartMessage(B_OK);
+				fLink.Attach<int32>(cursor->Token());
+			} else
+				fLink.StartMessage(status);
+
+			fLink.Flush();
 			break;
 		}
 
@@ -1288,8 +1307,8 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 
 			ServerCursor* cursor
 				= fDesktop->GetCursorManager().FindCursor(token);
-			if (cursor != NULL)
-				cursor->ReleaseReference();
+			if (cursor != NULL && cursor->OwningTeam() == fClientTeam)
+				fDesktop->GetCursorManager().RemoveCursor(cursor);
 
 			fDesktop->GetCursorManager().Unlock();
 

src/servers/app/ServerCursor.cpp

@@ -190,12 +190,3 @@ ServerCursor::AttachedToManager(CursorManager* manager)
 {
 	fManager = manager;
 }
-
-
-void
-ServerCursor::LastReferenceReleased()
-{
-	if (fManager == NULL || fManager->RemoveCursor(this))
-		delete this;
-}
-

src/servers/app/ServerCursor.h

@@ -52,9 +52,6 @@ public:
 			const uint8*		CursorData() const
 									{ return fCursorData; }
 
-protected:
-	virtual	void				LastReferenceReleased();
-
 private:
 	friend class CursorManager;