Bug 1054425 - cache2: leak in CacheFileMetadata::WriteMetadata, r=jduell
--- a/netwerk/cache2/CacheFileIOManager.cpp
+++ b/netwerk/cache2/CacheFileIOManager.cpp
@@ -1862,16 +1862,21 @@ CacheFileIOManager::Write(CacheFileHandl
LOG(("CacheFileIOManager::Write() [handle=%p, offset=%lld, count=%d, "
"validate=%d, listener=%p]", aHandle, aOffset, aCount, aValidate,
aCallback));
nsresult rv;
nsRefPtr<CacheFileIOManager> ioMan = gInstance;
if (aHandle->IsClosed() || !ioMan) {
+ if (!aCallback) {
+ // When no callback is provided, CacheFileIOManager is responsible for
+ // releasing the buffer. We must release it even in case of failure.
+ free(const_cast<char *>(aBuf));
+ }
return NS_ERROR_NOT_INITIALIZED;
}
nsRefPtr<WriteEvent> ev = new WriteEvent(aHandle, aOffset, aBuf, aCount,
aValidate, aCallback);
rv = ioMan->mIOThread->Dispatch(ev, CacheIOThread::WRITE);
NS_ENSURE_SUCCESS(rv, rv);
--- a/netwerk/cache2/CacheFileMetadata.cpp
+++ b/netwerk/cache2/CacheFileMetadata.cpp
@@ -255,43 +255,36 @@ CacheFileMetadata::WriteMetadata(uint32_
CacheHash::Hash32_t hash;
hash = CacheHash::Hash(mWriteBuf + sizeof(uint32_t),
p - mWriteBuf - sizeof(uint32_t));
NetworkEndian::writeUint32(mWriteBuf, hash);
NetworkEndian::writeUint32(p, aOffset);
p += sizeof(uint32_t);
- char * writeBuffer;
+ char * writeBuffer = mWriteBuf;
if (aListener) {
mListener = aListener;
- writeBuffer = mWriteBuf;
} else {
- // We are not going to pass |this| as callback to CacheFileIOManager::Write
- // so we must allocate a new buffer that will be released automatically when
- // write is finished. This is actually better than to let
- // CacheFileMetadata::OnDataWritten do the job, since when dispatching the
- // result from some reason fails during shutdown, we would unnecessarily leak
- // both this object and the buffer.
- writeBuffer = static_cast<char *>(moz_xmalloc(p - mWriteBuf));
- memcpy(writeBuffer, mWriteBuf, p - mWriteBuf);
+ // We are not going to pass |this| as a callback so the buffer will be
+ // released by CacheFileIOManager. Just null out mWriteBuf here.
+ mWriteBuf = nullptr;
}
- rv = CacheFileIOManager::Write(mHandle, aOffset, writeBuffer, p - mWriteBuf,
+ rv = CacheFileIOManager::Write(mHandle, aOffset, writeBuffer, p - writeBuffer,
true, aListener ? this : nullptr);
if (NS_FAILED(rv)) {
LOG(("CacheFileMetadata::WriteMetadata() - CacheFileIOManager::Write() "
"failed synchronously. [this=%p, rv=0x%08x]", this, rv));
mListener = nullptr;
- if (writeBuffer != mWriteBuf) {
- free(writeBuffer);
+ if (mWriteBuf) {
+ free(mWriteBuf);
+ mWriteBuf = nullptr;
}
- free(mWriteBuf);
- mWriteBuf = nullptr;
NS_ENSURE_SUCCESS(rv, rv);
}
DoMemoryReport(MemoryUsage());
return NS_OK;
}