* ServerApp's bitmap and picture handling was completely broken, as it ignored

concurrency as well as reference counting, causing occasional crashes and
  memory corruption.
* ServerPicture now subclasses Referenceable, and will notify its owner when
  it's going to be deleted. This might bring some regressions, although I
  couldn't spot anything wrong yet.
* ServerBitmap will now also notify its owner when it's going to be deleted as
  well.
* Switched from the former picture/bitmap lists to a std::map. This also solves
  the issue of not checking whether or not the bitmap/picture actually belongs
  to the ServerApp (before, all apps could access and delete all
  pictures/bitmaps)
* Introduced a new fMapLocker that guards the new maps.
* ServerWindow now uses GetBitmap()/GetPicture(), and gives up its reference
  after use.


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

src/servers/app/BitmapManager.cpp

@@ -220,6 +220,9 @@ BitmapManager::DeleteBitmap(ServerBitmap* bitmap)
 	if (bitmap->Overlay() != NULL)
 		fOverlays.RemoveItem(bitmap);
 
+	if (bitmap->Owner() != NULL)
+		bitmap->Owner()->BitmapRemoved(bitmap);
+
 	if (fBitmapList.RemoveItem(bitmap))
 		delete bitmap;
 }

src/servers/app/OffscreenServerWindow.cpp

@@ -24,6 +24,7 @@ OffscreenServerWindow::OffscreenServerWindow(const char *title, ServerApp *app,
 
 OffscreenServerWindow::~OffscreenServerWindow()
 {
+	fBitmap->Release();
 }
 
 

src/servers/app/ServerApp.cpp

@@ -97,6 +97,7 @@ ServerApp::ServerApp(Desktop* desktop, port_id clientReplyPort,
 	fClientTeam(clientTeam),
 	fWindowListLock("window list"),
 	fTemporaryDisplayModeChange(0),
+	fMapLocker("server app maps"),
 	fAppCursor(NULL),
 	fViewCursor(NULL),
 	fCursorHideLevel(0),
@@ -188,12 +189,20 @@ ServerApp::~ServerApp()
 		fWindowListLock.Lock();
 	}
 
-	for (int32 i = fBitmapList.CountItems(); i-- > 0;) {
+	fMapLocker.Lock();
-		gBitmapManager->DeleteBitmap((ServerBitmap*)fBitmapList.ItemAt(i));
+
+	while (!fBitmapMap.empty()) {
+		ServerBitmap* bitmap = fBitmapMap.begin()->second;
+
+		fBitmapMap.erase(fBitmapMap.begin());
+		bitmap->Release();
 	}
 
-	for (int32 i = fPictureList.CountItems(); i-- > 0;) {
+	while (!fPictureMap.empty()) {
-		delete fPictureList.ItemAt(i);
+		ServerPicture* picture = fPictureMap.begin()->second;
+
+		fPictureMap.erase(fPictureMap.begin());
+		picture->ReleaseReference();
 	}
 
 	fDesktop->GetCursorManager().DeleteCursors(fClientTeam);
@@ -243,37 +252,6 @@ ServerApp::Quit(sem_id shutdownSemaphore)
 }
 
 
-/*!	\brief Send a message to the ServerApp's BApplication
-	\param message The message to send
-*/
-void
-ServerApp::SendMessageToClient(BMessage* message) const
-{
-	status_t status = fHandlerMessenger.SendMessage(message, (BHandler*)NULL,
-		100000);
-	if (status != B_OK) {
-		syslog(LOG_ERR, "app %s send to client failed: %s\n", Signature(),
-			strerror(status));
-	}
-}
-
-
-bool
-ServerApp::_HasWindowUnderMouse()
-{
-	BAutolock locker(fWindowListLock);
-
-	for (int32 i = fWindowList.CountItems(); i-- > 0;) {
-		ServerWindow* serverWindow = fWindowList.ItemAt(i);
-
-		if (fDesktop->ViewUnderMouse(serverWindow->Window()) != B_NULL_TOKEN)
-			return true;
-	}
-
-	return false;
-}
-
-
 /*!	\brief Sets the ServerApp's active status
 	\param value The new status of the ServerApp.
 
@@ -302,6 +280,21 @@ ServerApp::Activate(bool value)
 }
 
 
+/*!	\brief Send a message to the ServerApp's BApplication
+	\param message The message to send
+*/
+void
+ServerApp::SendMessageToClient(BMessage* message) const
+{
+	status_t status = fHandlerMessenger.SendMessage(message, (BHandler*)NULL,
+		100000);
+	if (status != B_OK) {
+		syslog(LOG_ERR, "app %s send to client failed: %s\n", Signature(),
+			strerror(status));
+	}
+}
+
+
 void
 ServerApp::SetCurrentCursor(ServerCursor* cursor)
 {
@@ -330,6 +323,185 @@ ServerApp::CurrentCursor() const
 }
 
 
+bool
+ServerApp::AddWindow(ServerWindow* window)
+{
+	BAutolock locker(fWindowListLock);
+
+	return fWindowList.AddItem(window);
+}
+
+
+void
+ServerApp::RemoveWindow(ServerWindow* window)
+{
+	BAutolock locker(fWindowListLock);
+
+	fWindowList.RemoveItem(window);
+}
+
+
+bool
+ServerApp::InWorkspace(int32 index) const
+{
+	BAutolock locker(fWindowListLock);
+
+	// we could cache this, but then we'd have to recompute the cached
+	// value everytime a window has closed or changed workspaces
+
+	// TODO: support initial application workspace!
+
+	for (int32 i = fWindowList.CountItems(); i-- > 0;) {
+		ServerWindow* serverWindow = fWindowList.ItemAt(i);
+
+		const Window* window = serverWindow->Window();
+		if (window == NULL || window->IsOffscreenWindow())
+			continue;
+
+		// only normal and unhidden windows count
+
+		if (window->IsNormal() && !window->IsHidden()
+			&& window->InWorkspace(index))
+			return true;
+	}
+
+	return false;
+}
+
+
+uint32
+ServerApp::Workspaces() const
+{
+	uint32 workspaces = 0;
+
+	BAutolock locker(fWindowListLock);
+
+	// we could cache this, but then we'd have to recompute the cached
+	// value everytime a window has closed or changed workspaces
+
+	for (int32 i = fWindowList.CountItems(); i-- > 0;) {
+		ServerWindow* serverWindow = fWindowList.ItemAt(i);
+
+		const Window* window = serverWindow->Window();
+		if (window == NULL || window->IsOffscreenWindow())
+			continue;
+
+		// only normal and unhidden windows count
+
+		if (window->IsNormal() && !window->IsHidden())
+			workspaces |= window->Workspaces();
+	}
+
+	// TODO: add initial application workspace!
+	return workspaces;
+}
+
+
+/*!	\brief Acquires a reference of the desired bitmap, if available.
+	\param token ID token of the bitmap to find
+	\return The bitmap having that ID or NULL if not found
+*/
+ServerBitmap*
+ServerApp::GetBitmap(int32 token) const
+{
+	if (token < 1)
+		return NULL;
+
+	BAutolock _(fMapLocker);
+
+	ServerBitmap* bitmap = _FindBitmap(token);
+	if (bitmap == NULL)
+		return NULL;
+
+	bitmap->Acquire();
+
+	return bitmap;
+}
+
+
+bool
+ServerApp::BitmapAdded(ServerBitmap* bitmap)
+{
+	BAutolock _(fMapLocker);
+
+	try {
+		fBitmapMap.insert(std::make_pair(bitmap->Token(), bitmap));
+	} catch (std::bad_alloc& exception) {
+		return false;
+	}
+
+	return true;
+}
+
+
+void
+ServerApp::BitmapRemoved(ServerBitmap* bitmap)
+{
+	BAutolock _(fMapLocker);
+	fBitmapMap.erase(bitmap->Token());
+}
+
+
+ServerPicture*
+ServerApp::CreatePicture(const ServerPicture* original)
+{
+	ServerPicture* picture;
+	if (original != NULL)
+		picture = new(std::nothrow) ServerPicture(*original);
+	else
+		picture = new(std::nothrow) ServerPicture();
+
+	if (picture != NULL && !picture->SetOwner(this))
+		picture->ReleaseReference();
+
+	return picture;
+}
+
+
+ServerPicture*
+ServerApp::GetPicture(int32 token) const
+{
+	if (token < 1)
+		return NULL;
+
+	BAutolock _(fMapLocker);
+
+	ServerPicture* picture = _FindPicture(token);
+	if (picture == NULL)
+		return NULL;
+
+	picture->AcquireReference();
+
+	return picture;
+}
+
+
+bool
+ServerApp::PictureAdded(ServerPicture* picture)
+{
+	BAutolock _(fMapLocker);
+
+	try {
+		fPictureMap.insert(std::make_pair(picture->Token(), picture));
+	} catch (std::bad_alloc& exception) {
+		return false;
+	}
+
+	return true;
+}
+
+
+void
+ServerApp::PictureRemoved(ServerPicture* picture)
+{
+	BAutolock _(fMapLocker);
+	fPictureMap.erase(picture->Token());
+}
+
+
+// #pragma mark - private methods
+
+
 void
 ServerApp::_GetLooperName(char* name, size_t length)
 {
@@ -337,88 +509,6 @@ ServerApp::_GetLooperName(char* name, size_t length)
 }
 
 
-/*!	\brief The thread function ServerApps use to monitor messages
-*/
-void
-ServerApp::_MessageLooper()
-{
-	// Message-dispatching loop for the ServerApp
-
-	// First let's tell the client how to talk with us.
-	fLink.StartMessage(B_OK);
-	fLink.Attach<port_id>(fMessagePort);
-	fLink.Attach<area_id>(fDesktop->SharedReadOnlyArea());
-	fLink.Flush();
-
-	BPrivate::LinkReceiver &receiver = fLink.Receiver();
-
-	int32 code;
-	status_t err = B_OK;
-
-	while (!fQuitting) {
-		STRACE(("info: ServerApp::_MessageLooper() listening on port %ld.\n",
-			fMessagePort));
-
-		err = receiver.GetNextMessage(code, B_INFINITE_TIMEOUT);
-		if (err != B_OK || code == B_QUIT_REQUESTED) {
-			STRACE(("ServerApp: application seems to be gone...\n"));
-
-			// Tell desktop to quit us
-			BPrivate::LinkSender link(fDesktop->MessagePort());
-			link.StartMessage(AS_DELETE_APP);
-			link.Attach<thread_id>(Thread());
-			link.Flush();
-			break;
-		}
-
-		switch (code) {
-			case kMsgAppQuit:
-				// we receive this from our destructor on quit
-				fQuitting = true;
-				break;
-
-			case AS_QUIT_APP:
-			{
-				// This message is received only when the app_server is asked
-				// to shut down in test/debug mode. Of course, if we are testing
-				// while using AccelerantDriver, we do NOT want to shut down
-				// client applications. The server can be quit in this fashion
-				// through the driver's interface, such as closing the
-				// ViewDriver's window.
-
-				STRACE(("ServerApp %s:Server shutdown notification received\n",
-					Signature()));
-
-				// If we are using the real, accelerated version of the
-				// DrawingEngine, we do NOT want the user to be able shut down
-				// the server. The results would NOT be pretty
-#if TEST_MODE
-				BMessage pleaseQuit(B_QUIT_REQUESTED);
-				SendMessageToClient(&pleaseQuit);
-#endif
-				break;
-			}
-
-			default:
-				STRACE(("ServerApp %s: Got a Message to dispatch\n",
-					Signature()));
-				_DispatchMessage(code, receiver);
-				break;
-		}
-	}
-
-	// Quit() will send us a message; we're handling the exiting procedure
-	thread_id sender;
-	sem_id shutdownSemaphore;
-	receive_data(&sender, &shutdownSemaphore, sizeof(sem_id));
-
-	delete this;
-
-	if (shutdownSemaphore >= B_OK)
-		release_sem(shutdownSemaphore);
-}
-
-
 /*!	\brief Handler function for BApplication API messages
 	\param code Identifier code for the message. Equivalent to BMessage::what
 	\param buffer Any attachments
@@ -653,9 +743,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 			STRACE(("ServerApp %s: Create Bitmap (%.1fx%.1f)\n",
 				Signature(), frame.Width() + 1, frame.Height() + 1));
 
-			if (bitmap != NULL && fBitmapList.AddItem(bitmap)) {
+			if (bitmap != NULL && bitmap->SetOwner(this)) {
-				bitmap->SetOwner(this);
-
 				fLink.StartMessage(B_OK);
 				fLink.Attach<int32>(bitmap->Token());
 				fLink.Attach<uint8>(allocationFlags);
@@ -665,11 +753,11 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 				fLink.Attach<int32>(
 					fMemoryAllocator.AreaOffset(bitmap->AllocationCookie()));
 
-				if (allocationFlags & kFramebuffer)
+				if ((allocationFlags & kFramebuffer) != 0)
 					fLink.Attach<int32>(bitmap->BytesPerRow());
 			} else {
 				if (bitmap != NULL)
-					gBitmapManager->DeleteBitmap(bitmap);
+					bitmap->Release();
 
 				fLink.StartMessage(B_NO_MEMORY);
 			}
@@ -689,13 +777,17 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 			int32 token;
 			link.Read<int32>(&token);
 
-			ServerBitmap *bitmap = FindBitmap(token);
+			fMapLocker.Lock();
-			if (bitmap && fBitmapList.RemoveItem(bitmap)) {
+
+			ServerBitmap* bitmap = _FindBitmap(token);
+			if (bitmap != NULL) {
 				STRACE(("ServerApp %s: Deleting Bitmap %ld\n", Signature(),
 					token));
 
-				gBitmapManager->DeleteBitmap(bitmap);
+				bitmap->Release();
 			}
+
+			fMapLocker.Unlock();
 			break;
 		}
 		case AS_GET_BITMAP_OVERLAY_RESTRICTIONS:
@@ -707,13 +799,15 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 			if (link.Read<int32>(&token) != B_OK)
 				break;
 
-			ServerBitmap *bitmap = FindBitmap(token);
+			ServerBitmap* bitmap = GetBitmap(token);
 			if (bitmap != NULL) {
 				STRACE(("ServerApp %s: Get overlay restrictions for bitmap "
 					"%ld\n", Signature(), token));
 
 				status = fDesktop->HWInterface()->GetOverlayRestrictions(
 					bitmap->Overlay(), &restrictions);
+
+				bitmap->Release();
 			}
 
 			fLink.StartMessage(status);
@@ -752,7 +846,9 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 				for (int32 i = 0; i < subPicturesCount; i++) {
 					int32 token = -1;
 					link.Read<int32>(&token);
-					if (ServerPicture* subPicture = FindPicture(token))
+
+					// TODO: do we actually need another reference here?
+					if (ServerPicture* subPicture = GetPicture(token))
 						picture->NestPicture(subPicture);
 				}
 				status = picture->ImportData(link);
@@ -771,8 +867,13 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 		{
 			STRACE(("ServerApp %s: Delete Picture\n", Signature()));
 			int32 token;
-			if (link.Read<int32>(&token) == B_OK)
+			if (link.Read<int32>(&token) == B_OK) {
-				DeletePicture(token);
+				BAutolock _(fMapLocker);
+
+				ServerPicture* picture = _FindPicture(token);
+				if (picture != NULL)
+					picture->ReleaseReference();
+			}
 			break;
 		}
 
@@ -782,7 +883,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 			int32 token;
 			ServerPicture* original = NULL;
 			if (link.Read<int32>(&token) == B_OK)
-				original = FindPicture(token);
+				original = GetPicture(token);
 
 			if (original != NULL) {
 				ServerPicture* cloned = CreatePicture(original);
@@ -791,6 +892,8 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 					fLink.Attach<int32>(cloned->Token());
 				} else
 					fLink.StartMessage(B_NO_MEMORY);
+
+				original->ReleaseReference();
 			} else
 				fLink.StartMessage(B_BAD_VALUE);
 
@@ -803,10 +906,11 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 			STRACE(("ServerApp %s: Download Picture\n", Signature()));
 			int32 token;
 			link.Read<int32>(&token);
-			ServerPicture* picture = FindPicture(token);
+			ServerPicture* picture = GetPicture(token);
 			if (picture != NULL) {
 				picture->ExportData(fLink);
 					// ExportData() calls StartMessage() already
+				picture->ReleaseReference();
 			} else
 				fLink.StartMessage(B_ERROR);
 
@@ -2740,8 +2844,8 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 		case AS_READ_BITMAP:
 		{
 			STRACE(("ServerApp %s: AS_READ_BITMAP\n", Signature()));
-			int32 bitmapToken;
+			int32 token;
-			link.Read<int32>(&bitmapToken);
+			link.Read<int32>(&token);
 
 			bool drawCursor = true;
 			link.Read<bool>(&drawCursor);
@@ -2749,13 +2853,15 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 			BRect bounds;
 			link.Read<BRect>(&bounds);
 
-			ServerBitmap *bitmap = FindBitmap(bitmapToken);
+			ServerBitmap* bitmap = GetBitmap(token);
 			if (bitmap != NULL) {
 				if (fDesktop->GetDrawingEngine()->ReadBitmap(bitmap,
-					drawCursor, bounds) == B_OK) {
+						drawCursor, bounds) == B_OK) {
 					fLink.StartMessage(B_OK);
 				} else
 					fLink.StartMessage(B_BAD_VALUE);
+
+				gBitmapManager->DeleteBitmap(bitmap);
 			} else
 				fLink.StartMessage(B_BAD_VALUE);
 
@@ -2910,6 +3016,88 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
 }
 
 
+/*!	\brief The thread function ServerApps use to monitor messages
+*/
+void
+ServerApp::_MessageLooper()
+{
+	// Message-dispatching loop for the ServerApp
+
+	// First let's tell the client how to talk with us.
+	fLink.StartMessage(B_OK);
+	fLink.Attach<port_id>(fMessagePort);
+	fLink.Attach<area_id>(fDesktop->SharedReadOnlyArea());
+	fLink.Flush();
+
+	BPrivate::LinkReceiver &receiver = fLink.Receiver();
+
+	int32 code;
+	status_t err = B_OK;
+
+	while (!fQuitting) {
+		STRACE(("info: ServerApp::_MessageLooper() listening on port %ld.\n",
+			fMessagePort));
+
+		err = receiver.GetNextMessage(code, B_INFINITE_TIMEOUT);
+		if (err != B_OK || code == B_QUIT_REQUESTED) {
+			STRACE(("ServerApp: application seems to be gone...\n"));
+
+			// Tell desktop to quit us
+			BPrivate::LinkSender link(fDesktop->MessagePort());
+			link.StartMessage(AS_DELETE_APP);
+			link.Attach<thread_id>(Thread());
+			link.Flush();
+			break;
+		}
+
+		switch (code) {
+			case kMsgAppQuit:
+				// we receive this from our destructor on quit
+				fQuitting = true;
+				break;
+
+			case AS_QUIT_APP:
+			{
+				// This message is received only when the app_server is asked
+				// to shut down in test/debug mode. Of course, if we are testing
+				// while using AccelerantDriver, we do NOT want to shut down
+				// client applications. The server can be quit in this fashion
+				// through the driver's interface, such as closing the
+				// ViewDriver's window.
+
+				STRACE(("ServerApp %s:Server shutdown notification received\n",
+					Signature()));
+
+				// If we are using the real, accelerated version of the
+				// DrawingEngine, we do NOT want the user to be able shut down
+				// the server. The results would NOT be pretty
+#if TEST_MODE
+				BMessage pleaseQuit(B_QUIT_REQUESTED);
+				SendMessageToClient(&pleaseQuit);
+#endif
+				break;
+			}
+
+			default:
+				STRACE(("ServerApp %s: Got a Message to dispatch\n",
+					Signature()));
+				_DispatchMessage(code, receiver);
+				break;
+		}
+	}
+
+	// Quit() will send us a message; we're handling the exiting procedure
+	thread_id sender;
+	sem_id shutdownSemaphore;
+	receive_data(&sender, &shutdownSemaphore, sizeof(sem_id));
+
+	delete this;
+
+	if (shutdownSemaphore >= B_OK)
+		release_sem(shutdownSemaphore);
+}
+
+
 status_t
 ServerApp::_CreateWindow(int32 code, BPrivate::LinkReceiver& link,
 	port_id& clientReplyPort)
@@ -2960,7 +3148,7 @@ ServerApp::_CreateWindow(int32 code, BPrivate::LinkReceiver& link,
 	ServerWindow *window = NULL;
 
 	if (code == AS_CREATE_OFFSCREEN_WINDOW) {
-		ServerBitmap* bitmap = FindBitmap(bitmapToken);
+		ServerBitmap* bitmap = GetBitmap(bitmapToken);
 
 		if (bitmap != NULL) {
 			window = new (nothrow) OffscreenServerWindow(title, this,
@@ -2996,44 +3184,14 @@ ServerApp::_CreateWindow(int32 code, BPrivate::LinkReceiver& link,
 
 
 bool
-ServerApp::AddWindow(ServerWindow* window)
+ServerApp::_HasWindowUnderMouse()
 {
 	BAutolock locker(fWindowListLock);
 
-	return fWindowList.AddItem(window);
-}
-
-
-void
-ServerApp::RemoveWindow(ServerWindow* window)
-{
-	BAutolock locker(fWindowListLock);
-
-	fWindowList.RemoveItem(window);
-}
-
-
-bool
-ServerApp::InWorkspace(int32 index) const
-{
-	BAutolock locker(fWindowListLock);
-
-	// we could cache this, but then we'd have to recompute the cached
-	// value everytime a window has closed or changed workspaces
-
-	// TODO: support initial application workspace!
-
 	for (int32 i = fWindowList.CountItems(); i-- > 0;) {
 		ServerWindow* serverWindow = fWindowList.ItemAt(i);
 
-		const Window* window = serverWindow->Window();
+		if (fDesktop->ViewUnderMouse(serverWindow->Window()) != B_NULL_TOKEN)
-		if (window == NULL || window->IsOffscreenWindow())
-			continue;
-
-		// only normal and unhidden windows count
-
-		if (window->IsNormal() && !window->IsHidden()
-			&& window->InWorkspace(index))
 			return true;
 	}
 
@@ -3041,110 +3199,27 @@ ServerApp::InWorkspace(int32 index) const
 }
 
 
-uint32
-ServerApp::Workspaces() const
-{
-	uint32 workspaces = 0;
-
-	BAutolock locker(fWindowListLock);
-
-	// we could cache this, but then we'd have to recompute the cached
-	// value everytime a window has closed or changed workspaces
-
-	for (int32 i = fWindowList.CountItems(); i-- > 0;) {
-		ServerWindow* serverWindow = fWindowList.ItemAt(i);
-
-		const Window* window = serverWindow->Window();
-		if (window == NULL || window->IsOffscreenWindow())
-			continue;
-
-		// only normal and unhidden windows count
-
-		if (window->IsNormal() && !window->IsHidden())
-			workspaces |= window->Workspaces();
-	}
-
-	// TODO: add initial application workspace!
-	return workspaces;
-}
-
-
-int32
-ServerApp::CountBitmaps() const
-{
-	return fBitmapList.CountItems();
-}
-
-
-/*!	\brief Looks up a ServerApp's ServerBitmap in its list
-	\param token ID token of the bitmap to find
-	\return The bitmap having that ID or NULL if not found
-*/
 ServerBitmap*
-ServerApp::FindBitmap(int32 token) const
+ServerApp::_FindBitmap(int32 token) const
 {
-	// TODO: we need to make sure the bitmap is ours?!
+	ASSERT(fMapLock.IsLocked());
-	ServerBitmap* bitmap;
-	if (gTokenSpace.GetToken(token, kBitmapToken, (void**)&bitmap) == B_OK)
-		return bitmap;
 
-	return NULL;
+	BitmapMap::const_iterator iterator = fBitmapMap.find(token);
-}
+	if (iterator == fBitmapMap.end())
+		return NULL;
 
-
+	return iterator->second;
-int32
-ServerApp::CountPictures() const
-{
-	return fPictureList.CountItems();
 }
 
 
 ServerPicture*
-ServerApp::CreatePicture(const ServerPicture* original)
+ServerApp::_FindPicture(int32 token) const
 {
-	ServerPicture* picture;
+	ASSERT(fMapLock.IsLocked());
-	if (original != NULL)
-		picture = new(std::nothrow) ServerPicture(*original);
-	else
-		picture = new(std::nothrow) ServerPicture();
 
-	if (picture != NULL)
+	PictureMap::const_iterator iterator = fPictureMap.find(token);
-		fPictureList.AddItem(picture);
+	if (iterator == fPictureMap.end())
+		return NULL;
 
-	return picture;
+	return iterator->second;
-}
-
-
-ServerPicture*
-ServerApp::FindPicture(int32 token) const
-{
-	// TODO: we need to make sure the picture is ours?!
-	ServerPicture* picture;
-	if (gTokenSpace.GetToken(token, kPictureToken, (void**)&picture) == B_OK)
-		return picture;
-
-	return NULL;
-}
-
-
-bool
-ServerApp::DeletePicture(int32 token)
-{
-	ServerPicture* picture = FindPicture(token);
-	if (picture == NULL)
-		return false;
-
-	if (!fPictureList.RemoveItem(picture))
-		return false;
-
-	delete picture;
-
-	return true;
-}
-
-
-team_id
-ServerApp::ClientTeam() const
-{
-	return fClientTeam;
 }

src/servers/app/ServerApp.h

@@ -64,7 +64,7 @@ public:
 			void				SetCurrentCursor(ServerCursor* cursor);
 			ServerCursor*		CurrentCursor() const;
 
-			team_id				ClientTeam() const;
+			team_id				ClientTeam() const { return fClientTeam; }
 
 			const char*			Signature() const
 									{ return fSignature.String(); }
@@ -78,14 +78,15 @@ public:
 			int32				InitialWorkspace() const
 									{ return fInitialWorkspace; }
 
-			int32				CountBitmaps() const;
+			ServerBitmap*		GetBitmap(int32 token) const;
-			ServerBitmap*		FindBitmap(int32 token) const;
+			bool				BitmapAdded(ServerBitmap* bitmap);
+			void				BitmapRemoved(ServerBitmap* bitmap);
 
-			int32				CountPictures() const;
 			ServerPicture*		CreatePicture(
 									const ServerPicture* original = NULL);
-			ServerPicture*		FindPicture(int32 token) const;
+			ServerPicture*		GetPicture(int32 token) const;
-			bool				DeletePicture(int32 token);
+			bool				PictureAdded(ServerPicture* picture);
+			void				PictureRemoved(ServerPicture* picture);
 
 			Desktop*			GetDesktop() const { return fDesktop; }
 
@@ -94,17 +95,23 @@ public:
 			BPrivate::BTokenSpace& ViewTokens() { return fViewTokens; }
 
 private:
+	virtual	void				_GetLooperName(char* name, size_t size);
 	virtual	void				_DispatchMessage(int32 code,
 									BPrivate::LinkReceiver& link);
 	virtual	void				_MessageLooper();
-	virtual	void				_GetLooperName(char* name, size_t size);
 			status_t			_CreateWindow(int32 code,
 									BPrivate::LinkReceiver& link,
 									port_id& clientReplyPort);
 
 			bool				_HasWindowUnderMouse();
 
+			ServerBitmap*		_FindBitmap(int32 token) const;
+			ServerPicture*		_FindPicture(int32 token) const;
+
 private:
+	typedef std::map<int32, ServerBitmap*> BitmapMap;
+	typedef std::map<int32, ServerPicture*> PictureMap;
+
 			port_id				fMessagePort;
 			port_id				fClientReplyPort;
 									// our BApplication's event port
@@ -133,8 +140,9 @@ private:
 			// NOTE: Bitmaps and Pictures are stored globally, but ServerApps
 			// remember which ones they own so that they can destroy them when
 			// they quit.
-			BList				fBitmapList;
+	mutable	BLocker				fMapLocker;
-			BObjectList<ServerPicture> fPictureList;
+			BitmapMap			fBitmapMap;
+			PictureMap			fPictureMap;
 
 			ServerCursor*		fAppCursor;
 			ServerCursor*		fViewCursor;

src/servers/app/ServerBitmap.cpp

@@ -15,11 +15,14 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "BitmapManager.h"
 #include "ClientMemoryAllocator.h"
 #include "ColorConversion.h"
 #include "HWInterface.h"
 #include "InterfacePrivate.h"
 #include "Overlay.h"
+#include "ServerApp.h"
+
 
 using std::nothrow;
 using namespace BPrivate;
@@ -127,6 +130,13 @@ ServerBitmap::Acquire()
 }
 
 
+void
+ServerBitmap::Release()
+{
+	gBitmapManager->DeleteBitmap(this);
+}
+
+
 bool
 ServerBitmap::_Release()
 {
@@ -212,10 +222,18 @@ ServerBitmap::Overlay() const
 }
 
 
-void
+bool
 ServerBitmap::SetOwner(ServerApp* owner)
 {
-	fOwner = owner;
+	if (fOwner != NULL)
+		fOwner->BitmapRemoved(this);
+
+	if (owner != NULL && owner->BitmapAdded(this)) {
+		fOwner = owner;
+		return true;
+	}
+
+	return false;
 }
 
 

src/servers/app/ServerBitmap.h

@@ -29,11 +29,12 @@ class ServerApp;
 	all cursors. Every BBitmap has a shadow ServerBitmap object.
 */
 class ServerBitmap {
- public:
+public:
 	inline	bool			IsValid() const
 								{ return fBuffer != NULL; }
 
 			void			Acquire();
+			void			Release();
 
 	inline	uint8*			Bits() const
 								{ return fBuffer; }
@@ -68,7 +69,7 @@ class ServerBitmap {
 			void			SetOverlay(::Overlay* overlay);
 			::Overlay*		Overlay() const;
 
-			void			SetOwner(ServerApp* owner);
+			bool			SetOwner(ServerApp* owner);
 			ServerApp*		Owner() const;
 
 	//! Does a shallow copy of the bitmap passed to it
@@ -113,7 +114,7 @@ protected:
 };
 
 class UtilityBitmap : public ServerBitmap {
- public:
+public:
 							UtilityBitmap(BRect rect, color_space space,
 								uint32 flags, int32 bytesperline = -1,
 								screen_id screen = B_MAIN_SCREEN_ID);

src/servers/app/ServerPicture.cpp

@@ -460,12 +460,14 @@ static void
 draw_picture(View* view, BPoint where, int32 token)
 {
 	ServerPicture* picture
-		= view->Window()->ServerWindow()->App()->FindPicture(token);
+		= view->Window()->ServerWindow()->App()->GetPicture(token);
 	if (picture != NULL) {
 		view->SetDrawingOrigin(where);
 		view->PushState();
 		picture->Play(view);
 		view->PopState();
+
+		picture->ReleaseReference();
 	}
 }
 
@@ -781,7 +783,8 @@ ServerPicture::ServerPicture()
 	:
 	fFile(NULL),
 	fPictures(NULL),
-	fUsurped(NULL)
+	fUsurped(NULL),
+	fOwner(NULL)
 {
 	fToken = gTokenSpace.NewToken(kPictureToken, this);
 	fData = new(std::nothrow) BMallocIO();
@@ -795,7 +798,8 @@ ServerPicture::ServerPicture(const ServerPicture& picture)
 	fFile(NULL),
 	fData(NULL),
 	fPictures(NULL),
-	fUsurped(NULL)
+	fUsurped(NULL),
+	fOwner(NULL)
 {
 	fToken = gTokenSpace.NewToken(kPictureToken, this);
 
@@ -821,7 +825,8 @@ ServerPicture::ServerPicture(const char* fileName, int32 offset)
 	fFile(NULL),
 	fData(NULL),
 	fPictures(NULL),
-	fUsurped(NULL)
+	fUsurped(NULL),
+	fOwner(NULL)
 {
 	fToken = gTokenSpace.NewToken(kPictureToken, this);
 
@@ -848,10 +853,27 @@ ServerPicture::~ServerPicture()
 	delete fFile;
 	gTokenSpace.RemoveToken(fToken);
 
-	// We only delete the subpictures list, not the subpictures
+	if (fPictures != NULL) {
-	// themselves, since the ServerApp keeps them in a list and
+		for (int32 i = fPictures->CountItems(); i-- > 0;)
-	// will delete them on quit.
+			fPictures->ItemAt(i)->ReleaseReference();
-	delete fPictures;
+
+		delete fPictures;
+	}
+}
+
+
+bool
+ServerPicture::SetOwner(ServerApp* owner)
+{
+	if (fOwner != NULL)
+		fOwner->PictureRemoved(this);
+
+	if (owner != NULL && owner->PictureAdded(this)) {
+		fOwner = owner;
+		return true;
+	}
+
+	return false;
 }
 
 
@@ -1082,3 +1104,13 @@ ServerPicture::ExportData(BPrivate::PortLink& link)
 	fData->Seek(oldPosition, SEEK_SET);
 	return status;
 }
+
+
+void
+ServerPicture::LastReferenceReleased()
+{
+	if (fOwner != NULL)
+		fOwner->PictureRemoved(this);
+
+	delete this;
+}

src/servers/app/ServerPicture.h

@@ -14,6 +14,7 @@
 
 #include <ObjectList.h>
 #include <PictureDataWriter.h>
+#include <Referenceable.h>
 
 
 class ServerApp;
@@ -27,7 +28,7 @@ namespace BPrivate {
 class BList;
 
 
-class ServerPicture : public PictureDataWriter {
+class ServerPicture : public Referenceable, public PictureDataWriter {
 public:
 								ServerPicture();
 								ServerPicture(const ServerPicture& other);
@@ -36,6 +37,7 @@ public:
 								~ServerPicture();
 
 			int32				Token() { return fToken; }
+			bool				SetOwner(ServerApp* owner);
 
 			void				EnterStateChange();
 			void				ExitStateChange();
@@ -55,6 +57,9 @@ public:
 			status_t			ImportData(BPrivate::LinkReceiver& link);
 			status_t			ExportData(BPrivate::PortLink& link);
 
+protected:
+	virtual	void				LastReferenceReleased();
+
 private:
 	typedef BObjectList<ServerPicture> PictureList;
 
@@ -63,6 +68,7 @@ private:
 			BPositionIO*		fData;
 			PictureList*		fPictures;
 			ServerPicture*		fUsurped;
+			ServerApp*			fOwner;
 };
 
 #endif	// SERVER_PICTURE_H

src/servers/app/ServerWindow.cpp

@@ -1797,13 +1797,11 @@ fDesktop->LockSingleWindow();
 			rgb_color colorKey = {0};
 
 			if (status == B_OK) {
-				ServerBitmap* bitmap = fServerApp->FindBitmap(bitmapToken);
+				ServerBitmap* bitmap = fServerApp->GetBitmap(bitmapToken);
 				if (bitmapToken == -1 || bitmap != NULL) {
 					bool wasOverlay = fCurrentView->ViewBitmap() != NULL
 						&& fCurrentView->ViewBitmap()->Overlay() != NULL;
 
-					// TODO: this is a race condition: the bitmap could have been
-					//	deleted in the mean time!!
 					fCurrentView->SetViewBitmap(bitmap, srcRect, dstRect,
 						resizingMode, options);
 
@@ -1821,6 +1819,8 @@ fDesktop->LockSingleWindow();
 						bitmap->Overlay()->SetFlags(options);
 						colorKey = bitmap->Overlay()->Color();
 					}
+
+					bitmap->Release();
 				} else
 					status = B_BAD_VALUE;
 			}
@@ -1862,20 +1862,17 @@ fDesktop->LockSingleWindow();
 			if (link.Read<bool>(&inverse) != B_OK)
 				break;
 
-			// search for a picture with the specified token.
+			ServerPicture* picture = fServerApp->GetPicture(pictureToken);
-			ServerPicture *picture = fServerApp->FindPicture(pictureToken);
-			// TODO: Increase that picture's reference count.
-			// (~ allocate a picture)
 			if (picture == NULL)
 				break;
 
 			BRegion region;
 			// TODO: I think we also need the BView's token
 			// I think PictureToRegion would fit better into the View class (?)
-			if (PictureToRegion(picture, region, inverse, where) < B_OK)
+			if (PictureToRegion(picture, region, inverse, where) == B_OK)
-				break;
+				fCurrentView->SetUserClipping(&region);
 
-			fCurrentView->SetUserClipping(&region);
+			picture->ReleaseReference();
 			break;
 		}
 
@@ -1996,12 +1993,13 @@ fDesktop->LockSingleWindow();
 				if (link.Read(buffer, bufferSize) == B_OK
 					&& dragMessage.Unflatten(buffer) == B_OK) {
 						ServerBitmap* bitmap
-							= fServerApp->FindBitmap(bitmapToken);
+							= fServerApp->GetBitmap(bitmapToken);
 						// TODO: possible deadlock
 fDesktop->UnlockSingleWindow();
 						fDesktop->EventDispatcher().SetDragMessage(dragMessage,
 							bitmap, offset);
 fDesktop->LockSingleWindow();
+						bitmap->Release();
 				}
 				delete[] buffer;
 			}
@@ -2067,7 +2065,7 @@ fDesktop->LockSingleWindow();
 		{
 			DTRACE(("ServerWindow %s: Message AS_VIEW_BEGIN_PICTURE\n",
 				Title()));
-			ServerPicture *picture = App()->CreatePicture();
+			ServerPicture* picture = App()->CreatePicture();
 			picture->SyncState(fCurrentView);
 			fCurrentView->SetPicture(picture);
 			break;
@@ -2078,13 +2076,17 @@ fDesktop->LockSingleWindow();
 			DTRACE(("ServerWindow %s: Message AS_VIEW_APPEND_TO_PICTURE\n",
 				Title()));
 
-			int32 pictureToken;
+			int32 token;
-			link.Read<int32>(&pictureToken);
+			link.Read<int32>(&token);
-			ServerPicture *picture = App()->FindPicture(pictureToken);
+
-			if (picture)
+			ServerPicture* picture = App()->GetPicture(token);
+			if (picture != NULL)
 				picture->SyncState(fCurrentView);
+
 			fCurrentView->SetPicture(picture);
-				// we don't care if it's NULL
+
+			if (picture != NULL)
+				picture->ReleaseReference();
 			break;
 		}
 
@@ -2093,7 +2095,7 @@ fDesktop->LockSingleWindow();
 			DTRACE(("ServerWindow %s: Message AS_VIEW_END_PICTURE\n",
 				Title()));
 
-			ServerPicture *picture = fCurrentView->Picture();
+			ServerPicture* picture = fCurrentView->Picture();
 			if (picture != NULL) {
 				fCurrentView->SetPicture(NULL);
 				fLink.StartMessage(B_OK);
@@ -2263,9 +2265,8 @@ ServerWindow::_DispatchViewDrawingMessage(int32 code,
 				options |= B_FILTER_BITMAP_BILINEAR;
 #endif
 
-			ServerBitmap* bitmap = fServerApp->FindBitmap(info.bitmapToken);
+			ServerBitmap* bitmap = fServerApp->GetBitmap(info.bitmapToken);
-			if (bitmap) {
+			if (bitmap != NULL) {
-
 				DTRACE(("ServerWindow %s: Message AS_VIEW_DRAW_BITMAP: "
 					"View: %s, bitmap: %ld (size %ld x %ld), "
 					"BRect(%.1f, %.1f, %.1f, %.1f) -> "
@@ -2281,8 +2282,9 @@ ServerWindow::_DispatchViewDrawingMessage(int32 code,
 
 				drawingEngine->DrawBitmap(bitmap, info.bitmapRect,
 					info.viewRect, info.options);
-			}
 
+				bitmap->Release();
+			}
 			break;
 		}
 		case AS_STROKE_ARC:
@@ -2716,18 +2718,20 @@ ServerWindow::_DispatchViewDrawingMessage(int32 code,
 		case AS_VIEW_DRAW_PICTURE:
 		{
 			int32 token;
-			if (link.Read<int32>(&token) == B_OK) {
+			link.Read<int32>(&token);
-				BPoint where;
-				link.Read<BPoint>(&where);
 
-				ServerPicture *picture = App()->FindPicture(token);
+			BPoint where;
+			if (link.Read<BPoint>(&where) == B_OK) {
+				ServerPicture* picture = App()->GetPicture(token);
 				if (picture != NULL) {
 					fCurrentView->PushState();
+
 					fCurrentView->SetDrawingOrigin(where);
-					fCurrentView->PushState();
 					picture->Play(fCurrentView);
+
 					fCurrentView->PopState();
-					fCurrentView->PopState();
+
+					picture->ReleaseReference();
 				}
 			}
 			break;
@@ -3115,7 +3119,7 @@ ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
 			ViewDrawBitmapInfo info;
 			link.Read<ViewDrawBitmapInfo>(&info);
 
-			ServerBitmap *bitmap = App()->FindBitmap(info.bitmapToken);
+			ServerBitmap* bitmap = App()->GetBitmap(info.bitmapToken);
 			if (bitmap == NULL)
 				break;
 
@@ -3124,23 +3128,26 @@ ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
 				bitmap->ColorSpace(), info.options, bitmap->Bits(),
 				bitmap->BitsLength());
 
+			bitmap->Release();
 			break;
 		}
 
 		case AS_VIEW_DRAW_PICTURE:
 		{
 			int32 token;
-			if (link.Read<int32>(&token) == B_OK) {
+			link.Read<int32>(&token);
-				BPoint where;
-				link.Read<BPoint>(&where);
 
-				ServerPicture *pictureToDraw = App()->FindPicture(token);
+			BPoint where;
-				if (picture != NULL) {
+			if (link.Read<BPoint>(&where) == B_OK) {
+				ServerPicture* pictureToDraw = App()->GetPicture(token);
+				if (pictureToDraw != NULL) {
 					// We need to make a copy of the picture, since it can
 					// change after it has been drawn
-					ServerPicture *copy = App()->CreatePicture(pictureToDraw);
+					ServerPicture* copy = App()->CreatePicture(pictureToDraw);
 					picture->NestPicture(copy);
 					picture->WriteDrawPicture(where, copy->Token());
+
+					pictureToDraw->ReleaseReference();
 				}
 			}
 			break;
@@ -3183,21 +3190,26 @@ ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
 
 		case AS_VIEW_APPEND_TO_PICTURE:
 		{
-			int32 pictureToken;
+			int32 token;
-			link.Read<int32>(&pictureToken);
+			link.Read<int32>(&token);
-			ServerPicture *appendPicture = App()->FindPicture(pictureToken);
+
-			if (appendPicture) {
+			ServerPicture* appendPicture = App()->GetPicture(token);
+			if (appendPicture != NULL) {
 				//picture->SyncState(fCurrentView);
 				appendPicture->Usurp(picture);
 			}
+
 			fCurrentView->SetPicture(appendPicture);
-				// we don't care if it's NULL
+
+			if (appendPicture != NULL)
+				appendPicture->ReleaseReference();
 			break;
 		}
 
 		case AS_VIEW_END_PICTURE:
 		{
-			ServerPicture *steppedDown = picture->StepDown();
+			ServerPicture* steppedDown = picture->StepDown();
+
 			fCurrentView->SetPicture(steppedDown);
 			fLink.StartMessage(B_OK);
 			fLink.Attach<int32>(picture->Token());

src/servers/app/View.cpp

@@ -154,7 +154,7 @@ View::InitCheck() const
 {
 	if (fDrawState == NULL)
 		return B_NO_MEMORY;
-	
+
 	return B_OK;
 }
 
@@ -1291,7 +1291,7 @@ View::SetEventMask(uint32 eventMask, uint32 options)
 
 
 void
-View::SetCursor(ServerCursor *cursor)
+View::SetCursor(ServerCursor* cursor)
 {
 	if (cursor == fCursor)
 		return;
@@ -1307,9 +1307,18 @@ View::SetCursor(ServerCursor *cursor)
 
 
 void
-View::SetPicture(ServerPicture *picture)
+View::SetPicture(ServerPicture* picture)
 {
+	if (picture == fPicture)
+		return;
+
+	if (fPicture != NULL)
+		fPicture->ReleaseReference();
+
 	fPicture = picture;
+
+	if (fPicture != NULL)
+		fPicture->AcquireReference();
 }