diff --git a/src/servers/app/ServerApp.cpp b/src/servers/app/ServerApp.cpp index 96b21d5211..8892b55619 100644 --- a/src/servers/app/ServerApp.cpp +++ b/src/servers/app/ServerApp.cpp @@ -197,7 +197,7 @@ ServerApp::~ServerApp() _DeleteBitmap(fBitmapMap.begin()->second); while (!fPictureMap.empty()) - RemovePicture(fPictureMap.begin()->second); + fPictureMap.begin()->second->SetOwner(NULL); fDesktop->GetCursorManager().DeleteCursors(fClientTeam); @@ -453,11 +453,14 @@ ServerApp::GetPicture(int32 token) const } +/*! To be called only by ServerPicture itself.*/ bool ServerApp::AddPicture(ServerPicture* picture) { BAutolock _(fMapLocker); + ASSERT(picture->Owner() == NULL); + try { fPictureMap.insert(std::make_pair(picture->Token(), picture)); } catch (std::bad_alloc& exception) { @@ -468,11 +471,14 @@ ServerApp::AddPicture(ServerPicture* picture) } +/*! To be called only by ServerPicture itself.*/ void ServerApp::RemovePicture(ServerPicture* picture) { BAutolock _(fMapLocker); + ASSERT(picture->Owner() == this); + fPictureMap.erase(picture->Token()); picture->ReleaseReference(); } @@ -834,8 +840,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) int32 token = -1; link.Read(&token); - // TODO: do we actually need another reference here? - if (ServerPicture* subPicture = GetPicture(token)) + if (ServerPicture* subPicture = _FindPicture(token)) picture->NestPicture(subPicture); } status = picture->ImportData(link); @@ -859,7 +864,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) ServerPicture* picture = _FindPicture(token); if (picture != NULL) - RemovePicture(picture); + picture->SetOwner(NULL); } break; } diff --git a/src/servers/app/ServerPicture.cpp b/src/servers/app/ServerPicture.cpp index 8d41ad80bc..05746503f7 100644 --- a/src/servers/app/ServerPicture.cpp +++ b/src/servers/app/ServerPicture.cpp @@ -931,16 +931,27 @@ ServerPicture::~ServerPicture() bool ServerPicture::SetOwner(ServerApp* owner) { + if (owner == fOwner) + return true; + + // Acquire an extra reference, since calling RemovePicture() + // May remove the last reference and then we will self-destruct right then. + // Setting fOwner to NULL would access free'd memory. If owner is another + // ServerApp, it's expected to already have a reference of course. + Reference _(this); + if (fOwner != NULL) fOwner->RemovePicture(this); - if (owner != NULL && owner->AddPicture(this)) { - fOwner = owner; - return true; - } - fOwner = NULL; - return false; + if (owner == NULL) + return true; + + if (!owner->AddPicture(this)) + return false; + + fOwner = owner; + return true; } @@ -1107,6 +1118,7 @@ ServerPicture::NestPicture(ServerPicture* picture) if (fPictures == NULL || !fPictures->AddItem(picture)) return false; + picture->AcquireReference(); return true; } diff --git a/src/servers/app/ServerPicture.h b/src/servers/app/ServerPicture.h index 7ad2225f02..92416202e9 100644 --- a/src/servers/app/ServerPicture.h +++ b/src/servers/app/ServerPicture.h @@ -38,6 +38,7 @@ public: int32 Token() { return fToken; } bool SetOwner(ServerApp* owner); + ServerApp* Owner() const { return fOwner; } bool ReleaseClientReference();