Closed Bug 10262 Opened 25 years ago Closed 24 years ago

Memory Leak in orkinHeap::Alloc(nsIMdbEnv *,DWORD,void * *)

Categories

(MailNews Core :: Database, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: skasinathan, Assigned: Bienvenu)

References

Details

(Keywords: memory-footprint, memory-leak)

I ran Purify on Mailnews code of the windows debug build dated 7/20/99. Looks like there are quite a bit of Memory Leaks in orkinHeap::Alloc(nsIMdbEnv *,DWORD,void * *). This leak has occured in the last 3/4 Purify runs. Here is the Stack trace. [W] MLK: Memory leak of 744 bytes from 1 block allocated in orkinHeap::Alloc(nsIMdbEnv *,DWORD,void * *) Distribution of leaked blocks 744 bytes from 1 block of 744 bytes (0x064a69b0) allocation number 353439 Allocation location new(UINT) [new.cpp:23] orkinHeap::Alloc(nsIMdbEnv *,DWORD,void * *) [orkinHeap.cpp:55] { MORK_USED_1(ev); mdb_err outErr = 0; => void* block = new char[ inSize ]; if ( !block ) outErr = morkEnv_kOutOfMemoryError; morkNode::MakeNew(UINT,nsIMdbHeap&,morkEnv *) [morkNode.cpp:163] void* node = 0; if ( &ioHeap ) { => ioHeap.Alloc(ev->AsMdbEnv(), inSize, (void **) &node); if ( !node ) ev->OutOfMemoryError(); } morkNode::new(UINT,nsIMdbHeap&,morkEnv *) [morkNode.h:141] public: // morkNode memory management operators void* operator new(size_t inSize, nsIMdbHeap& ioHeap, morkEnv* ev) => { return morkNode::MakeNew(inSize, ioHeap, ev); } void operator delete(void* ioAddress) { morkNode::OnDeleteAssert(ioAddress); } morkThumb::Make_LargeCommit(morkEnv *,nsIMdbHeap *,morkStore *) [morkThumb.cpp:261] if ( outThumb ) { morkWriter* writer = new(*ioHeap, ev) => morkWriter(ev, morkUsage::kHeap, ioHeap, ioStore, file, ioHeap); if ( writer ) { writer->mWriter_CommitGroupIdentity = orkinStore::SessionCommit(nsIMdbEnv *,nsIMdbThumb * *) [orkinStore.cpp:1113] morkFile* file = store->mStore_File; if ( file && file->Length(ev) > 128 && store->mStore_CanWriteIncremental ) { => thumb = morkThumb::Make_LargeCommit(ev, heap, store); } else { nsMsgDatabase::Commit(int) [nsMsgDatabase.cpp:685] break; case nsMsgDBCommitType::kSessionCommit: // comment out until persistence works. => err = m_mdbStore->SessionCommit(GetEnv(), &commitThumb); break; case nsMsgDBCommitType::kCompressCommit: err = m_mdbStore->CompressCommit(GetEnv(), &commitThumb); nsMsgDatabase::CloseMDB(int) [nsMsgDatabase.cpp:629] NS_IMETHODIMP nsMsgDatabase::CloseMDB(PRBool commit) { if (commit) => Commit(nsMsgDBCommitType::kSessionCommit); return(NS_OK); } nsMsgDatabase::Close(int) [nsMsgDatabase.cpp:712] NS_IMETHODIMP nsMsgDatabase::Close(PRBool forceCommit /* = TRUE */) { => return CloseMDB(forceCommit); } const char *kMsgHdrsScope = "ns:msg:db:row:scope:msgs:all"; // scope for all headers table nsMsgDBFolder::~nsMsgDBFolder(void) [nsMsgDBFolder.cpp:62] if(mDatabase) { mDatabase->RemoveListener(this); => mDatabase->Close(PR_TRUE); } } nsMsgLocalMailFolder::~nsMsgLocalMailFolder(void) [nsLocalMailFolder.cpp:113] { if (mPath) delete mPath; => } NS_IMPL_ADDREF_INHERITED(nsMsgLocalMailFolder, nsMsgFolder) NS_IMPL_RELEASE_INHERITED(nsMsgLocalMailFolder, nsMsgFolder) nsMsgLocalMailFolder::`scalar deleting destructor'(UINT) [msglocal.dll] nsRDFResource::Release(void) [nsRDFResource.cpp:60] } NS_IMPL_ADDREF(nsRDFResource) => NS_IMPL_RELEASE(nsRDFResource) NS_IMETHODIMP nsRDFResource::QueryInterface(REFNSIID aIID, void** aResult) nsMsgFolder::Release(void) [nsMsgFolder.cpp:90] } NS_IMPL_ADDREF_INHERITED(nsMsgFolder, nsRDFResource) => NS_IMPL_RELEASE_INHERITED(nsMsgFolder, nsRDFResource) NS_IMETHODIMP nsMsgFolder::QueryInterface(REFNSIID aIID, void** aInstancePtr) { nsMsgLocalMailFolder::Release(void) [nsLocalMailFolder.cpp:116] } NS_IMPL_ADDREF_INHERITED(nsMsgLocalMailFolder, nsMsgFolder) => NS_IMPL_RELEASE_INHERITED(nsMsgLocalMailFolder, nsMsgFolder) NS_IMETHODIMP nsMsgLocalMailFolder::QueryInterface(REFNSIID aIID, void** aInstancePtr)
QA Contact: lchiang → suresh
changing QA Contact to my name.
Status: NEW → ASSIGNED
I guess calling Release is not sufficient. I'll have to CutStrongRef and see if that fixes it.
If that doesn't fix it, can you reassign this to davidmc, suresh? thanks.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
fixed, I hope, by calling CutStrongRef.
Status: RESOLVED → REOPENED
Assignee: bienvenu → davidmc
Status: REOPENED → NEW
I could see this Memory Leak occuring in Today's build. I pulled the tree around 11.00 AM. You can see my Purify log postings at www.mozilla.org/mailnews/purify. Reassigning to davidmc.
Resolution: FIXED → ---
clearing resolution
Can you verify my interpretation of what that report says? Is the morkWriter instance the one being leaked?
I think so. If you do a search for phrase 'morkWriter' you will see some leaks. Ex: morkWriter::MakeWriterStream(morkEnv *) [morkWriter.cpp:227] and morkWriter::WriteMore(morkEnv *) [morkWriter.cpp:327]. morkThumb::DoMore_Commit(morkEnv *) [morkThumb.cpp:465] Correct me if I am wrong. Thanks.
If I search what? (I have not been examining any Purify logs at all, because I only want to look at what I can see using 4.5. I use a Mac, and I have no expectation that I will have a good experience if I try to use 5.0 with an XML format that requires that I tease the product into using correct DTDs etc.)
If you look through the reports filed at http://www.mozilla.org/mailnews/purify/. Yes, you will need to use the 5.0 browser, but it should be working on the Mac. If it's not, then we should be filing bugs on it to make sure that it is. Once you get the browser running you should only have to click on the correct Purify file in order to see it. You shouldn't need anything else.
Sorry my 5.0 build doesn't work yet, and that's what I wasted yesterday and today fixing, but it's not done yet. I was not joking about expecting the Mac not to work, and about expressing my disinterest in discovering this empirically. Thanks for verifying that's what Suresh meant for the search location.
come to my cube. you can use my windows box to view the purify log until you mac is working again.
Or you could come to my cube. Or you could try installing one of the daily 5.0 mac builds and run that. Or, you could download m8, which is reasonably stable - http://www.mozilla.org/projects/seamonkey/release-notes/
Those are all great offers; I will look at the leak report after I get some traction on my transition to abstract Mork files. Most likely I can fix any leak of morkWriter alone (or any other single object instead of entire clusters of related objects) by looking at refcounting in my sources, because it's probably in Mork. It's only likely above Mork when entire clumps of related things are leaked together, which suggests a parent root was not released.
David, if possible, please assign a target milestone for this bug (as to which milestone you plan to fix this bug in). This will help to know how many bugs are planned for each milestone to be fixed. Thanks. If you need to know the milestones and dates, pls let me know. I'll find out for you.
Target Milestone: M12
If this can be looked at sooner it would be great, but features are more important now. Setting M12.
I think David Bienvenu checked in a fix, and I reviewed his change, but I can't verify what's checked in at the moment because I'm still out on a mission to swap file infrastructure. I will append David's patch here so someone might see whether this is in the tree, and then a new purify report might show whether the leak is gone. Index: morkThumb.cpp =================================================================== RCS file: /cvsroot/mozilla/mailnews/db/mork/src/morkThumb.cpp,v retrieving revision 1.4 diff -c -r1.4 morkThumb.cpp *** morkThumb.cpp 1999/07/14 15:52:10 1.4 --- morkThumb.cpp 1999/08/01 17:00:29 *************** *** 268,273 **** --- 268,275 ---- morkStore::SlotStrongStore(ioStore, ev, &outThumb->mThumb_Store); morkFile::SlotStrongFile(file, ev, &outThumb->mThumb_File); morkWriter::SlotStrongWriter(writer, ev, &outThumb->mThumb_Writer); + // writer no longer holds on to this. + writer->CutStrongRef(ev); } } } David McCusker wrote: > > David Bienvenu wrote: > > > > Attached is a fix I checked in for a memory leak in Mork. > > Let me know if I've messed it > > up, but it seems like the "write" thing to do :-) > > Yes, that works. I think even better is to replace > morkWriter::SlotStrongWriter(writer, ev, &outThumb->mThumb_Writer); > with > outThumb->mThumb_Writer = writer; > > in order to simply transfer ownership to thumb. > Basically my mistake was ignoring that new() was a generator of > a strong ref, and I was leaking it. I've done that before, too. > > David
Blocks: 11091
(target milestone is M11 or M12 - add to mail beta tracking bug)
Status: NEW → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
I believe this was fixed a while ago.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
davidmc, If you take a look at my recent purify logs at www.mozilla.org/mailnews/purify, particularly '18-November 1999 POP' , approximately 22KB to 25KB is leaked from 'orkinHeap'. So, reopening the bug.
clearing Resolution.
Mr. Hofmann seems to think that this bug doesn't have a target fix milestone and that it's a crasher. Seems like the report says M12 but doesn't say anything about crasher. Anyway, since it's been reopened are we planning on fixing this for M12?
Whiteboard: not a crasher. move to m13?
this is not a crasher, it's just a memory leak.
Yea, I know. Mr. Hofmann sent out the wrong bug report. It's acutally 20078
can this move out to M13?
Target Milestone: M12 → M14
moving to m14
Assuming this is one leak of 744 bytes, we have bigger fish to fry. M16
Target Milestone: M14 → M16
Keywords: mlk
these are mine now, I guess.
Assignee: davidmc → bienvenu
Status: REOPENED → NEW
accepting
Status: NEW → ASSIGNED
Not M16 stopper. Marking M17.
Target Milestone: M16 → M17
moving to future.
Target Milestone: M17 → M20
adding nsbeta3 keyword
Keywords: nsbeta3
since david nominated this, I'm moving milestone to M18 which corresponds to nsbeta3 nominations.
Target Milestone: M20 → M18
Keywords: footprint
Whiteboard: not a crasher. move to m13?
I haven't see this leak in recent leak reports. Please note that there are other bugs filed on leaks that have orkinHeap::Alloc at the very bottom - this bug is for the particular stack trace posted, and I believe has been fixed for a while.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago24 years ago
Resolution: --- → FIXED
Old, old bug. We've got newer bugs logged.
Status: RESOLVED → VERIFIED
QA Contact: suresh → stephend
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.