* Fixed race conditions in the server's bitmap/picture handling: the objects

are now removed from the maps as soon as the client deletes them. This also
  makes the "client reference" mechanism superfluous I introduced earlier.
* ServerApp::SetCurrentCursor() must always call Desktop::SetCursor(), since it
  is also called whenever the current application changes. This fixes the cursor
  almost never changing.
* Renamed ServerPicture::Usurp()/StepDown() to PushPicture(), and PopPicture().
* Also, they now acquire a reference to the picture in question (ie. the picture
  you get from PopPicture() also owns a reference you need to free).
* ServerApp::CreatePicture() may fail, too. This case is now handled in the code
  that calls it.
* Previously, the ServerWindow tried to process up to 70 messages in one go.
  That obviously caused bug #4709. Now, we have the additional requirement to
  not hold the desktop lock for longer than 25 ms. I haven't tested it with
  Kaleidoscope yet, though.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@35472 a95241bf-73f2-0310-859d-f6bbb57e9c96
This commit is contained in:
Axel Dörfler
2010-02-15 14:50:53 +00:00
parent aeb689782c
commit 87e7b978aa
7 changed files with 147 additions and 184 deletions
+48 -50
View File
@@ -191,19 +191,11 @@ ServerApp::~ServerApp()
fMapLocker.Lock();
while (!fBitmapMap.empty()) {
ServerBitmap* bitmap = fBitmapMap.begin()->second;
while (!fBitmapMap.empty())
_DeleteBitmap(fBitmapMap.begin()->second);
fBitmapMap.erase(fBitmapMap.begin());
bitmap->ReleaseReference();
}
while (!fPictureMap.empty()) {
ServerPicture* picture = fPictureMap.begin()->second;
fPictureMap.erase(fPictureMap.begin());
picture->ReleaseReference();
}
while (!fPictureMap.empty())
RemovePicture(fPictureMap.begin()->second);
fDesktop->GetCursorManager().DeleteCursors(fClientTeam);
@@ -298,16 +290,15 @@ ServerApp::SendMessageToClient(BMessage* message) const
void
ServerApp::SetCurrentCursor(ServerCursor* cursor)
{
if (fViewCursor == cursor)
return;
if (fViewCursor != cursor) {
if (fViewCursor)
fViewCursor->ReleaseReference();
if (fViewCursor)
fViewCursor->ReleaseReference();
fViewCursor = cursor;
fViewCursor = cursor;
if (fViewCursor)
fViewCursor->AcquireReference();
if (fViewCursor)
fViewCursor->AcquireReference();
}
fDesktop->SetCursor(CurrentCursor());
}
@@ -419,29 +410,6 @@ ServerApp::GetBitmap(int32 token) const
}
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)
{
@@ -477,7 +445,7 @@ ServerApp::GetPicture(int32 token) const
bool
ServerApp::PictureAdded(ServerPicture* picture)
ServerApp::AddPicture(ServerPicture* picture)
{
BAutolock _(fMapLocker);
@@ -492,10 +460,12 @@ ServerApp::PictureAdded(ServerPicture* picture)
void
ServerApp::PictureRemoved(ServerPicture* picture)
ServerApp::RemovePicture(ServerPicture* picture)
{
BAutolock _(fMapLocker);
fPictureMap.erase(picture->Token());
picture->ReleaseReference();
}
@@ -743,7 +713,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 && bitmap->SetOwner(this)) {
if (bitmap != NULL && _AddBitmap(bitmap)) {
fLink.StartMessage(B_OK);
fLink.Attach<int32>(bitmap->Token());
fLink.Attach<uint8>(allocationFlags);
@@ -770,8 +740,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
STRACE(("ServerApp %s: received BBitmap delete request\n",
Signature()));
// Delete a bitmap's allocated memory
// Attached Data:
// 1) int32 token
int32 token;
@@ -784,7 +752,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
STRACE(("ServerApp %s: Deleting Bitmap %ld\n", Signature(),
token));
bitmap->ReleaseClientReference();
_DeleteBitmap(bitmap);
}
fMapLocker.Unlock();
@@ -833,6 +801,8 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
break;
}
// Picture ops
case AS_CREATE_PICTURE:
{
// TODO: Maybe rename this to AS_UPLOAD_PICTURE ?
@@ -872,7 +842,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
ServerPicture* picture = _FindPicture(token);
if (picture != NULL)
picture->ReleaseClientReference();
RemovePicture(picture);
}
break;
}
@@ -3199,6 +3169,34 @@ ServerApp::_HasWindowUnderMouse()
}
bool
ServerApp::_AddBitmap(ServerBitmap* bitmap)
{
BAutolock _(fMapLocker);
try {
fBitmapMap.insert(std::make_pair(bitmap->Token(), bitmap));
} catch (std::bad_alloc& exception) {
return false;
}
bitmap->SetOwner(this);
return true;
}
void
ServerApp::_DeleteBitmap(ServerBitmap* bitmap)
{
ASSERT(fMapLock.IsLocked());
gBitmapManager->BitmapRemoved(bitmap);
fBitmapMap.erase(bitmap->Token());
bitmap->ReleaseReference();
}
ServerBitmap*
ServerApp::_FindBitmap(int32 token) const
{
+6 -5
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2001-2009, Haiku.
* Copyright 2001-2010, Haiku.
* Distributed under the terms of the MIT License.
*
* Authors:
@@ -79,14 +79,12 @@ public:
{ return fInitialWorkspace; }
ServerBitmap* GetBitmap(int32 token) const;
bool BitmapAdded(ServerBitmap* bitmap);
void BitmapRemoved(ServerBitmap* bitmap);
ServerPicture* CreatePicture(
const ServerPicture* original = NULL);
ServerPicture* GetPicture(int32 token) const;
bool PictureAdded(ServerPicture* picture);
void PictureRemoved(ServerPicture* picture);
bool AddPicture(ServerPicture* picture);
void RemovePicture(ServerPicture* picture);
Desktop* GetDesktop() const { return fDesktop; }
@@ -105,7 +103,10 @@ private:
bool _HasWindowUnderMouse();
bool _AddBitmap(ServerBitmap* bitmap);
void _DeleteBitmap(ServerBitmap* bitmap);
ServerBitmap* _FindBitmap(int32 token) const;
ServerPicture* _FindPicture(int32 token) const;
private:
+14 -49
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2001-2009, Haiku.
* Copyright 2001-2010, Haiku.
* Distributed under the terms of the MIT License.
*
* Authors:
@@ -28,8 +28,7 @@ using std::nothrow;
using namespace BPrivate;
/*!
A word about memory housekeeping and why it's implemented this way:
/*! A word about memory housekeeping and why it's implemented this way:
The reason why this looks so complicated is to optimize the most common
path (bitmap creation from the application), and don't cause any further
@@ -47,8 +46,7 @@ using namespace BPrivate;
*/
/*!
\brief Constructor called by the BitmapManager (only).
/*! \brief Constructor called by the BitmapManager (only).
\param rect Size of the bitmap.
\param space Color space of the bitmap
\param flags Various bitmap flags to tweak the bitmap as defined in Bitmap.h
@@ -65,7 +63,6 @@ ServerBitmap::ServerBitmap(BRect rect, color_space space, uint32 flags,
fAllocationCookie(NULL),
fOverlay(NULL),
fBuffer(NULL),
fReferenceCount(1),
// WARNING: '1' is added to the width and height.
// Same is done in FBBitmap subclass, so if you
// modify here make sure to do the same under
@@ -75,8 +72,7 @@ ServerBitmap::ServerBitmap(BRect rect, color_space space, uint32 flags,
fBytesPerRow(0),
fSpace(space),
fFlags(flags),
fOwner(NULL),
fHasClientReference(true)
fOwner(NULL)
// fToken is initialized (if used) by the BitmapManager
{
int32 minBytesPerRow = get_bytes_per_row(space, fWidth);
@@ -92,8 +88,7 @@ ServerBitmap::ServerBitmap(const ServerBitmap* bitmap)
fAllocationCookie(NULL),
fOverlay(NULL),
fBuffer(NULL),
fReferenceCount(1),
fHasClientReference(false)
fOwner(NULL)
{
if (bitmap) {
fWidth = bitmap->fWidth;
@@ -101,14 +96,12 @@ ServerBitmap::ServerBitmap(const ServerBitmap* bitmap)
fBytesPerRow = bitmap->fBytesPerRow;
fSpace = bitmap->fSpace;
fFlags = bitmap->fFlags;
fOwner = bitmap->fOwner;
} else {
fWidth = 0;
fHeight = 0;
fBytesPerRow = 0;
fSpace = B_NO_COLOR_SPACE;
fFlags = 0;
fOwner = NULL;
}
}
@@ -200,18 +193,10 @@ ServerBitmap::Overlay() const
}
bool
void
ServerBitmap::SetOwner(ServerApp* owner)
{
if (fOwner != NULL)
fOwner->BitmapRemoved(this);
if (owner != NULL && owner->BitmapAdded(this)) {
fOwner = owner;
return true;
}
return false;
fOwner = owner;
}
@@ -222,18 +207,6 @@ ServerBitmap::Owner() const
}
bool
ServerBitmap::ReleaseClientReference()
{
if (!fHasClientReference)
return false;
fHasClientReference = false;
ReleaseReference();
return true;
}
void
ServerBitmap::PrintToStream()
{
@@ -242,30 +215,21 @@ ServerBitmap::PrintToStream()
}
void
ServerBitmap::LastReferenceReleased()
{
if (fOwner != NULL)
fOwner->BitmapRemoved(this);
gBitmapManager->BitmapRemoved(this);
delete this;
}
// #pragma mark -
UtilityBitmap::UtilityBitmap(BRect rect, color_space space, uint32 flags,
int32 bytesperline, screen_id screen)
: ServerBitmap(rect, space, flags, bytesperline, screen)
int32 bytesPerRow, screen_id screen)
:
ServerBitmap(rect, space, flags, bytesPerRow, screen)
{
AllocateBuffer();
}
UtilityBitmap::UtilityBitmap(const ServerBitmap* bitmap)
: ServerBitmap(bitmap)
:
ServerBitmap(bitmap)
{
AllocateBuffer();
@@ -276,7 +240,8 @@ UtilityBitmap::UtilityBitmap(const ServerBitmap* bitmap)
UtilityBitmap::UtilityBitmap(const uint8* alreadyPaddedData, uint32 width,
uint32 height, color_space format)
: ServerBitmap(BRect(0, 0, width - 1, height - 1), format, 0)
:
ServerBitmap(BRect(0, 0, width - 1, height - 1), format, 0)
{
AllocateBuffer();
if (Bits())
+2 -8
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2001-2009, Haiku.
* Copyright 2001-2010, Haiku.
* Distributed under the terms of the MIT License.
*
* Authors:
@@ -69,11 +69,9 @@ public:
void SetOverlay(::Overlay* overlay);
::Overlay* Overlay() const;
bool SetOwner(ServerApp* owner);
void SetOwner(ServerApp* owner);
ServerApp* Owner() const;
bool ReleaseClientReference();
//! Does a shallow copy of the bitmap passed to it
inline void ShallowCopy(const ServerBitmap *from);
@@ -95,8 +93,6 @@ protected:
ServerBitmap(const ServerBitmap* bmp);
virtual ~ServerBitmap();
virtual void LastReferenceReleased();
void AllocateBuffer();
protected:
@@ -104,7 +100,6 @@ protected:
void* fAllocationCookie;
::Overlay* fOverlay;
uint8* fBuffer;
int32 fReferenceCount;
int32 fWidth;
int32 fHeight;
@@ -114,7 +109,6 @@ protected:
ServerApp* fOwner;
int32 fToken;
bool fHasClientReference;
};
class UtilityBitmap : public ServerBitmap {
+43 -41
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2001-2009, Haiku.
* Copyright 2001-2010, Haiku.
* Distributed under the terms of the MIT License.
*
* Authors:
@@ -783,9 +783,8 @@ ServerPicture::ServerPicture()
:
fFile(NULL),
fPictures(NULL),
fUsurped(NULL),
fOwner(NULL),
fHasClientReference(true)
fPushed(NULL),
fOwner(NULL)
{
fToken = gTokenSpace.NewToken(kPictureToken, this);
fData = new(std::nothrow) BMallocIO();
@@ -799,9 +798,8 @@ ServerPicture::ServerPicture(const ServerPicture& picture)
fFile(NULL),
fData(NULL),
fPictures(NULL),
fUsurped(NULL),
fOwner(NULL),
fHasClientReference(false)
fPushed(NULL),
fOwner(NULL)
{
fToken = gTokenSpace.NewToken(kPictureToken, this);
@@ -827,9 +825,8 @@ ServerPicture::ServerPicture(const char* fileName, int32 offset)
fFile(NULL),
fData(NULL),
fPictures(NULL),
fUsurped(NULL),
fOwner(NULL),
fHasClientReference(true)
fPushed(NULL),
fOwner(NULL)
{
fToken = gTokenSpace.NewToken(kPictureToken, this);
@@ -852,16 +849,26 @@ ServerPicture::ServerPicture(const char* fileName, int32 offset)
ServerPicture::~ServerPicture()
{
ASSERT(fOwner == NULL);
delete fData;
delete fFile;
gTokenSpace.RemoveToken(fToken);
if (fPictures != NULL) {
for (int32 i = fPictures->CountItems(); i-- > 0;)
fPictures->ItemAt(i)->ReleaseReference();
for (int32 i = fPictures->CountItems(); i-- > 0;) {
ServerPicture* picture = fPictures->ItemAt(i);
picture->SetOwner(NULL);
picture->ReleaseReference();
}
delete fPictures;
}
if (fPushed != NULL) {
fPushed->SetOwner(NULL);
fPushed->ReleaseReference();
}
}
@@ -869,29 +876,18 @@ bool
ServerPicture::SetOwner(ServerApp* owner)
{
if (fOwner != NULL)
fOwner->PictureRemoved(this);
fOwner->RemovePicture(this);
if (owner != NULL && owner->PictureAdded(this)) {
if (owner != NULL && owner->AddPicture(this)) {
fOwner = owner;
return true;
}
fOwner = NULL;
return false;
}
bool
ServerPicture::ReleaseClientReference()
{
if (!fHasClientReference)
return false;
fHasClientReference = false;
ReleaseReference();
return true;
}
void
ServerPicture::EnterStateChange()
{
@@ -1014,22 +1010,38 @@ ServerPicture::Play(View* view)
}
/*! Acquires a reference to the pushed picture.
*/
void
ServerPicture::Usurp(ServerPicture* picture)
ServerPicture::PushPicture(ServerPicture* picture)
{
fUsurped = picture;
if (fPushed != NULL)
debugger("already pushed a picture");
fPushed = picture;
fPushed->AcquireReference();
}
/*! Returns a reference with the popped picture.
*/
ServerPicture*
ServerPicture::StepDown()
ServerPicture::PopPicture()
{
ServerPicture* old = fUsurped;
fUsurped = NULL;
ServerPicture* old = fPushed;
fPushed = NULL;
return old;
}
void
ServerPicture::AppendPicture(ServerPicture* picture)
{
// A pushed picture is the same as an appended one
PushPicture(picture);
}
bool
ServerPicture::NestPicture(ServerPicture* picture)
{
@@ -1119,13 +1131,3 @@ ServerPicture::ExportData(BPrivate::PortLink& link)
fData->Seek(oldPosition, SEEK_SET);
return status;
}
void
ServerPicture::LastReferenceReleased()
{
if (fOwner != NULL)
fOwner->PictureRemoved(this);
delete this;
}
+7 -9
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2001-2009, Haiku.
* Copyright 2001-2010, Haiku.
* Distributed under the terms of the MIT License.
*
* Authors:
@@ -49,9 +49,10 @@ public:
void Play(View* view);
void Usurp(ServerPicture* newPicture);
ServerPicture* StepDown();
void PushPicture(ServerPicture* picture);
ServerPicture* PopPicture();
void AppendPicture(ServerPicture* picture);
bool NestPicture(ServerPicture* picture);
off_t DataLength() const;
@@ -59,19 +60,16 @@ public:
status_t ImportData(BPrivate::LinkReceiver& link);
status_t ExportData(BPrivate::PortLink& link);
protected:
virtual void LastReferenceReleased();
private:
typedef BObjectList<ServerPicture> PictureList;
typedef BObjectList<ServerPicture> PictureList;
int32 fToken;
BFile* fFile;
BPositionIO* fData;
PictureList* fPictures;
ServerPicture* fUsurped;
ServerPicture* fPushed;
ServerApp* fOwner;
bool fHasClientReference;
};
#endif // SERVER_PICTURE_H
+27 -22
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2001-2009, Haiku.
* Copyright 2001-2010, Haiku.
* Distributed under the terms of the MIT License.
*
* Authors:
@@ -2066,8 +2066,10 @@ fDesktop->LockSingleWindow();
DTRACE(("ServerWindow %s: Message AS_VIEW_BEGIN_PICTURE\n",
Title()));
ServerPicture* picture = App()->CreatePicture();
picture->SyncState(fCurrentView);
fCurrentView->SetPicture(picture);
if (picture != NULL) {
picture->SyncState(fCurrentView);
fCurrentView->SetPicture(picture);
}
break;
}
@@ -2756,9 +2758,9 @@ ServerWindow::_DispatchViewDrawingMessage(int32 code,
bool
ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver& link)
{
ServerPicture *picture = fCurrentView->Picture();
ServerPicture* picture = fCurrentView->Picture();
if (picture == NULL)
return false;
@@ -2778,7 +2780,6 @@ ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
BRect rect;
link.Read<BRect>(&rect);
picture->WriteInvertRect(rect);
break;
}
@@ -3092,13 +3093,12 @@ ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
link.Read<int32>(&opCount);
link.Read<int32>(&ptCount);
uint32 *opList = new(nothrow) uint32[opCount];
BPoint *ptList = new(nothrow) BPoint[ptCount];
uint32* opList = new(std::nothrow) uint32[opCount];
BPoint* ptList = new(std::nothrow) BPoint[ptCount];
if (opList != NULL && ptList != NULL
&& link.Read(opList, opCount * sizeof(uint32)) >= B_OK
&& link.Read(ptList, ptCount * sizeof(BPoint)) >= B_OK) {
// This might seem a bit weird, but under R5, the shapes
// This might seem a bit weird, but under BeOS, the shapes
// are always offset by the current pen location
BPoint penLocation
= fCurrentView->CurrentState()->PenLocation();
@@ -3108,9 +3108,9 @@ ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
const bool fill = (code == AS_FILL_SHAPE);
picture->WriteDrawShape(opCount, opList, ptCount, ptList, fill);
}
delete[] opList;
delete[] ptList;
break;
}
@@ -3175,16 +3175,17 @@ ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
// we are supposed to clear the clipping region
picture->WriteClearClipping();
}
break;
}
case AS_VIEW_BEGIN_PICTURE:
{
ServerPicture *newPicture = App()->CreatePicture();
newPicture->Usurp(picture);
newPicture->SyncState(fCurrentView);
fCurrentView->SetPicture(newPicture);
ServerPicture* newPicture = App()->CreatePicture();
if (newPicture != NULL) {
newPicture->PushPicture(picture);
newPicture->SyncState(fCurrentView);
fCurrentView->SetPicture(newPicture);
}
break;
}
@@ -3196,7 +3197,7 @@ ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
ServerPicture* appendPicture = App()->GetPicture(token);
if (appendPicture != NULL) {
//picture->SyncState(fCurrentView);
appendPicture->Usurp(picture);
appendPicture->AppendPicture(picture);
}
fCurrentView->SetPicture(appendPicture);
@@ -3208,9 +3209,11 @@ ServerWindow::_DispatchPictureMessage(int32 code, BPrivate::LinkReceiver &link)
case AS_VIEW_END_PICTURE:
{
ServerPicture* steppedDown = picture->StepDown();
ServerPicture* poppedPicture = picture->PopPicture();
fCurrentView->SetPicture(poppedPicture);
if (poppedPicture != NULL)
poppedPicture->ReleaseReference();
fCurrentView->SetPicture(steppedDown);
fLink.StartMessage(B_OK);
fLink.Attach<int32>(picture->Token());
fLink.Flush();
@@ -3301,6 +3304,7 @@ ServerWindow::_MessageLooper()
#endif
int32 messagesProcessed = 0;
bigtime_t processingStart = system_time();
bool lockedDesktop = false;
bool needsAllWindowsLocked = false;
@@ -3384,8 +3388,9 @@ ServerWindow::_MessageLooper()
fDesktop->UnlockAllWindows();
// Only process up to 70 waiting messages at once (we have the
// Desktop locked)
if (!receiver.HasMessages() || ++messagesProcessed > 70) {
// Desktop locked), but don't hold the lock longer than 25 ms
if (!receiver.HasMessages() || ++messagesProcessed > 70
|| system_time() - processingStart > 25000) {
if (lockedDesktop)
fDesktop->UnlockSingleWindow();
break;