AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix a memory leak in SkGPipeCanvas.

Browse Source 
In cross process mode (currently unused but tested on the bots),
SkGPipeCanvas has a tangle of circular references, which prevents
it from being deleted. Break the chain of references.

R=junov@chromium.org

Review URL: https://codereview.chromium.org/69633003

git-svn-id: http://skia.googlecode.com/svn/trunk@12237 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
scroggo@google.com 2013-11-12 14:32:38 +00:00
parent d6bea606c6
commit 59c3ab637b
1 changed files with 71 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

118
src/pipe/SkGPipeWrite.cpp
View File

@ -167,32 +167,63 @@ public:
/////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////
/**
* If SkBitmaps are to be flattened to send to the reader, this class is
* provided to the SkBitmapHeap to tell the SkGPipeCanvas to do so.
*/
class BitmapShuttle : public SkBitmapHeap::ExternalStorage {
public:
BitmapShuttle(SkGPipeCanvas*);
~BitmapShuttle();
virtual bool insert(const SkBitmap& bitmap, int32_t slot) SK_OVERRIDE;
/**
* Remove the SkGPipeCanvas used for insertion. After this, calls to
* insert will crash.
*/
void removeCanvas();
private:
SkGPipeCanvas* fCanvas;
};
///////////////////////////////////////////////////////////////////////////////
class SkGPipeCanvas : public SkCanvas { class SkGPipeCanvas : public SkCanvas {
public: public:
SkGPipeCanvas(SkGPipeController*, SkWriter32*, uint32_t flags, SkGPipeCanvas(SkGPipeController*, SkWriter32*, uint32_t flags,
uint32_t width, uint32_t height); uint32_t width, uint32_t height);
virtual ~SkGPipeCanvas(); virtual ~SkGPipeCanvas();
void finish() { /**
if (!fDone) { * Called when nothing else is to be written to the stream. Any repeated
if (this->needOpBytes()) { * calls are ignored.
this->writeOp(kDone_DrawOp); *
this->doNotify(); * @param notifyReaders Whether to send a message to the reader(s) that
if (shouldFlattenBitmaps(fFlags)) { * the writer is through sending commands. Should generally be true,
// In this case, a BitmapShuttle is reffed by the SkBitmapHeap * unless there is an error which prevents further messages from
// and refs this canvas. Unref the SkBitmapHeap to end the * being sent.
// circular reference. When shouldFlattenBitmaps is false, */
// there is no circular reference, so the SkBitmapHeap can be void finish(bool notifyReaders) {
// safely unreffed in the destructor. if (fDone) {
fBitmapHeap->unref(); return;
// This eliminates a similar circular reference (Canvas owns
// the FlattenableHeap which holds a ref to the SkBitmapHeap).
fFlattenableHeap.setBitmapStorage(NULL);
fBitmapHeap = NULL;
}
}
fDone = true;
} }
if (notifyReaders && this->needOpBytes()) {
this->writeOp(kDone_DrawOp);
this->doNotify();
}
if (shouldFlattenBitmaps(fFlags)) {
// The following circular references exist:
// fFlattenableHeap -> fWriteBuffer -> fBitmapStorage -> fExternalStorage -> fCanvas
// fBitmapHeap -> fExternalStorage -> fCanvas
// fFlattenableHeap -> fBitmapStorage -> fExternalStorage -> fCanvas
// Break them all by destroying the final link to this SkGPipeCanvas.
fBitmapShuttle->removeCanvas();
}
fDone = true;
} }
void flushRecording(bool detachCurrentBlock); void flushRecording(bool detachCurrentBlock);
@ -306,9 +337,11 @@ private:
// if a new SkFlatData was added when in cross process mode // if a new SkFlatData was added when in cross process mode
void flattenFactoryNames(); void flattenFactoryNames();
FlattenableHeap fFlattenableHeap; FlattenableHeap fFlattenableHeap;
FlatDictionary fFlatDictionary; FlatDictionary fFlatDictionary;
int fCurrFlatIndex[kCount_PaintFlats]; SkAutoTUnref<BitmapShuttle> fBitmapShuttle;
int fCurrFlatIndex[kCount_PaintFlats];
int flattenToIndex(SkFlattenable* obj, PaintFlats); int flattenToIndex(SkFlattenable* obj, PaintFlats);
// Common code used by drawBitmap*. Behaves differently depending on the // Common code used by drawBitmap*. Behaves differently depending on the
@ -390,24 +423,6 @@ int SkGPipeCanvas::flattenToIndex(SkFlattenable* obj, PaintFlats paintflat) {
/////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////
/**
* If SkBitmaps are to be flattened to send to the reader, this class is
* provided to the SkBitmapHeap to tell the SkGPipeCanvas to do so.
*/
class BitmapShuttle : public SkBitmapHeap::ExternalStorage {
public:
BitmapShuttle(SkGPipeCanvas*);
~BitmapShuttle();
virtual bool insert(const SkBitmap& bitmap, int32_t slot) SK_OVERRIDE;
private:
SkGPipeCanvas* fCanvas;
};
///////////////////////////////////////////////////////////////////////////////
#define MIN_BLOCK_SIZE (16 * 1024) #define MIN_BLOCK_SIZE (16 * 1024)
#define BITMAPS_TO_KEEP 5 #define BITMAPS_TO_KEEP 5
#define FLATTENABLES_TO_KEEP 10 #define FLATTENABLES_TO_KEEP 10
@ -440,9 +455,8 @@ SkGPipeCanvas::SkGPipeCanvas(SkGPipeController* controller,
} }
if (shouldFlattenBitmaps(flags)) { if (shouldFlattenBitmaps(flags)) {
BitmapShuttle* shuttle = SkNEW_ARGS(BitmapShuttle, (this)); fBitmapShuttle.reset(SkNEW_ARGS(BitmapShuttle, (this)));
fBitmapHeap = SkNEW_ARGS(SkBitmapHeap, (shuttle, BITMAPS_TO_KEEP)); fBitmapHeap = SkNEW_ARGS(SkBitmapHeap, (fBitmapShuttle.get(), BITMAPS_TO_KEEP));
shuttle->unref();
} else { } else {
fBitmapHeap = SkNEW_ARGS(SkBitmapHeap, fBitmapHeap = SkNEW_ARGS(SkBitmapHeap,
(BITMAPS_TO_KEEP, controller->numberOfReaders())); (BITMAPS_TO_KEEP, controller->numberOfReaders()));
@ -456,7 +470,7 @@ SkGPipeCanvas::SkGPipeCanvas(SkGPipeController* controller,
} }
SkGPipeCanvas::~SkGPipeCanvas() { SkGPipeCanvas::~SkGPipeCanvas() {
this->finish(); this->finish(true);
SkSafeUnref(fFactorySet); SkSafeUnref(fFactorySet);
SkSafeUnref(fBitmapHeap); SkSafeUnref(fBitmapHeap);
} }
@ -474,7 +488,8 @@ bool SkGPipeCanvas::needOpBytes(size_t needed) {
size_t blockSize = SkMax32(MIN_BLOCK_SIZE, needed); size_t blockSize = SkMax32(MIN_BLOCK_SIZE, needed);
void* block = fController->requestBlock(blockSize, &fBlockSize); void* block = fController->requestBlock(blockSize, &fBlockSize);
if (NULL == block) { if (NULL == block) {
fDone = true; // Do not notify the readers, which would call this function again.
this->finish(false);
return false; return false;
} }
SkASSERT(SkIsAlign4(fBlockSize)); SkASSERT(SkIsAlign4(fBlockSize));
@ -1179,7 +1194,7 @@ SkCanvas* SkGPipeWriter::startRecording(SkGPipeController* controller, uint32_t
void SkGPipeWriter::endRecording() { void SkGPipeWriter::endRecording() {
if (fCanvas) { if (fCanvas) {
fCanvas->finish(); fCanvas->finish(true);
fCanvas->unref(); fCanvas->unref();
fCanvas = NULL; fCanvas = NULL;
} }
@ -1211,9 +1226,18 @@ BitmapShuttle::BitmapShuttle(SkGPipeCanvas* canvas) {
} }
BitmapShuttle::~BitmapShuttle() { BitmapShuttle::~BitmapShuttle() {
fCanvas->unref(); this->removeCanvas();
} }
bool BitmapShuttle::insert(const SkBitmap& bitmap, int32_t slot) { bool BitmapShuttle::insert(const SkBitmap& bitmap, int32_t slot) {
SkASSERT(fCanvas != NULL);
return fCanvas->shuttleBitmap(bitmap, slot); return fCanvas->shuttleBitmap(bitmap, slot);
} }
void BitmapShuttle::removeCanvas() {
if (NULL == fCanvas) {
return;
}
fCanvas->unref();
fCanvas = NULL;
}