Closed
Bug 33697
Opened 24 years ago
Closed 23 years ago
leaking mork stuff out of nsGlobalHistory::OpenDB()
Categories
(Core Graveyard :: History: Global, defect, P3)
Core Graveyard
History: Global
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: bruce, Assigned: Bienvenu)
Details
(Keywords: memory-leak)
This happens on mozilla-bin startup/shutdown for me. This afternoon's build, Solaris 2.6, gcc 2.7.2.3, etc. MLK: 64 bytes leaked at 0xaed500 This memory was allocated from: malloc [rtlib.o] __bUiLtIn_nEw [libxpcom.so] __builtin_new [rtlib.o] orkinHeap::Alloc(nsIMdbEnv*,unsigned long,void**) [orkinHeap.cpp:75] morkPool::NewHandle(morkEnv*,unsigned long,morkZone*) [morkPool.cpp:176] morkEnv::NewHandle(unsigned long) [morkEnv.h:145] orkinFile::MakeFile(morkEnv*,morkFile*) [orkinFile.cpp:79] MORK_ASSERT(isEnv); if ( isEnv ) { => morkHandleFace* face = ev->NewHandle(sizeof(orkinFile)); if ( face ) return new(face) orkinFile(ev, face, ioObject); else morkFile::AcquireFileHandle(morkEnv*) [morkFile.cpp:144] f->AddStrongRef(ev->AsMdbEnv()); else // need new handle? { => f = orkinFile::MakeFile(ev, this); mObject_Handle = f; } if ( f ) orkinFactory::OpenOldFile(nsIMdbEnv*,nsIMdbHeap*,const char*,unsigned char,nsIMdbFile**) [orkinFactory.cpp:315] morkFile* file = morkFile::OpenOldFile(ev, ioHeap, inFilePath, inFrozen); if ( file ) { => outFile = file->AcquireFileHandle(ev); file->CutStrongRef(ev); } nsGlobalHistory::OpenDB() [nsGlobalHistory.cpp:1665] nsMdbPtr<nsIMdbFile> oldFile(mEnv); // ensures file is released err = factory->OpenOldFile(mEnv, dbHeap, dbfile.GetNativePathCString(), => dbFrozen, getter_Acquires(oldFile)); if ((err != 0) || !oldFile) return NS_ERROR_FAILURE; nsGlobalHistory::Init() [nsGlobalHistory.cpp:1610] NS_NewGlobalHistory(nsISupports*,const nsID&,void**) [nsGlobalHistory.cpp:533] nsGenericFactory::CreateInstance(nsISupports*,const nsID&,void**) [nsGenericFactory.cpp:47] nsComponentManagerImpl::CreateInstance(const nsID&,nsISupports*,const nsID&,void**) [nsComponentManager.cpp:1156] nsComponentManager::CreateInstance(const nsID&,nsISupports*,const nsID&,void**) [nsRepository.cpp:81] nsServiceManagerImpl::GetService(const nsID&,const nsID&,nsISupports**,nsIShutdownListener*) [nsServiceManager.cpp:293] nsServiceManagerImpl::GetService(const char*,const nsID&,nsISupports**,nsIShutdownListener*) [nsServiceManager.cpp:431] nsServiceManager::GetService(const char*,const nsID&,nsISupports**,nsIShutdownListener*) [nsServiceManager.cpp:544] nsGetServiceByProgID::operator ()(const nsID&,void**)const [nsServiceManager.cpp:63] nsCOMPtr<nsIGlobalHistory>::assign_from_helper(const nsCOMPtr_helper&,const nsID&) [nsCOMPtr.h:825] nsCOMPtr<nsIGlobalHistory>::nsCOMPtr<nsIGlobalHistory>(const nsCOMPtr_helper&) [nsCOMPtr.h:527] nsDocShell::SetTitle(const unsigned short*) [nsDocShell.cpp:1538] nsHTMLDocument::SetTitle(const nsString&) [nsHTMLDocument.cpp:796] HTMLContentSink::DidBuildModel(int) [nsHTMLContentSink.cpp:2285] CNavDTD::DidBuildModel(unsigned int,int,nsIParser*,nsIContentSink*) [CNavDTD.cpp:641] nsParser::DidBuildModel(unsigned int) [nsParser.cpp:726] nsParser::ResumeParse(int,int) [nsParser.cpp:1214] nsParser::OnStopRequest(nsIChannel*,nsISupports*,unsigned int,const unsigned short*) [nsParser.cpp:1652] nsDocumentOpenInfo::OnStopRequest(nsIChannel*,nsISupports*,unsigned int,const unsigned short*) [nsURILoader.cpp:276] nsInputStreamChannel::OnStopRequest(nsIChannel*,nsISupports*,unsigned int,const unsigned short*) [nsInputStreamChannel.cpp:366]
Comment 2•24 years ago
|
||
david[b|mc]: is this the "mork was designed to leak" problem?
Assignee | ||
Comment 3•24 years ago
|
||
I don't think files are built for leakage. but I could be wrong.
Comment 4•24 years ago
|
||
bruce: is nsGlobalHistory leaking? could you check the bloat logs? thanks.
Updated•24 years ago
|
Keywords: mlk
Summary: MLK: mork-stuff out of nsGlobalHistory::OpenDB() → mork-stuff out of nsGlobalHistory::OpenDB()
Reporter | ||
Comment 5•24 years ago
|
||
To the best of my knowledge, it was the Mork environments which were designed to leak and I'm not reporting those in bug reports. If I'm wrong in that, I'd love to know it too! (and I'll look to see if Global History leaks when my current run finishes)
Reporter | ||
Comment 6•24 years ago
|
||
XPCOM_MEM_LEAK_LOG says that nope, we aren't leaking nsGlobalHistory during mozilla-bin startup/shutdown.
The store db returned from OpenOldFile() holds a reference to the file indefinitely until the store gets closed, which is done manually or when the store uses (ie strong refs) hit zero. This might all be irrelevant. It looks like the orkinFile instance gets leaked. This is a 'handle' object holding a reference to the actual file. I think I am allocating all the handles from a free list of such handles in the environment's pool, so this might be an instance of "mork envs are designed to leak" since the handle free lists would also be leaked. However, when you are all done with a mork env, you can explicitly close it, which will free all memory being referenced by the env, and this would free any objects like free list handles. So while env instances are designed to leak, we need not leak things referenced by an env since you can close them explicitly.
Reporter | ||
Comment 8•24 years ago
|
||
So, to test this, I can close mEnv in nsGlobalHistory::~nsGlobalHistory(). mEnv is of type nsIMdbEnv however, which doesn't export the call to morkEnv::CloseEnv(). morkEnv::CloseEnv() is called by morkEnv::CloseMorkNode() as well. How do I get from an nsIMdbEnv to a morkEnv?
One should call nsIMdbObject::CloseMdbObject() instead, since nsIMdbEnv is a subclass and all objects are closable this way. Note this method perversely requires an instance of nsIMdbEnv as an argument. I expect this might work if you passed the env to itself when closing, but I don't remember thinking about such a pathological case, so I'm a bit leery. You could instead have around another env instance used only to close other env instances. I know, it sounds silly, but it should be well-behaved.
Reporter | ||
Comment 10•24 years ago
|
||
Thanks. I will try this out and test it under Purify and see what evil results from passing mEnv to mEnv->CloseMdbObject(); I will post results here. Another neat trick would be to add in some Purify API calls to let Purify know about the memory handling with the free lists. I don't have that type of energy this month or next though.
Reporter | ||
Comment 11•24 years ago
|
||
Placing a mEnv->CloseMdbObject(mEnv); in nsGlobalHistory::~nsGlobalHistory() didn't seem to free up any additional memory. It didn't create any bad errors under Purify either though. David: Do I need any CutStrongRef's or anything else to help make it go away?
Comment 12•24 years ago
|
||
Hmm, you could put a breakpoint in morkEnv::CloseEnv() to see if you reach it, and then one in morkPool::ClosePool() to see if you reach that tool. If you reach the former but not the latter, that would suggest the pool has more strong references to it (perhaps from a zone?). I think it makes sense to add a line like the following to morkEnv::CloseEnv(): if ( mEnv_HandlePool ) mEnv_HandlePool->CloseMorkNode(ev); early in the method.
Comment 13•24 years ago
|
||
bruce: how are we doing on this one? should i reassign to you?
Reporter | ||
Comment 14•24 years ago
|
||
I'll take it. I haven't had a lot of time, and probably won't until next week though.
Assignee: waterson → bruce
Target Milestone: --- → M16
Reporter | ||
Comment 15•24 years ago
|
||
I'll accept this, but I won't have time to look at it for a long while.
Status: NEW → ASSIGNED
Target Milestone: M16 → M18
Reporter | ||
Comment 16•24 years ago
|
||
This sounds like a nice project for some other genius. I haven't any interest in fixing leaks at the current time.
Assignee: bruce → radha
Status: ASSIGNED → NEW
Updated•24 years ago
|
Status: NEW → ASSIGNED
Updated•24 years ago
|
Target Milestone: M18 → Future
Updated•24 years ago
|
Status: NEW → ASSIGNED
Summary: mork-stuff out of nsGlobalHistory::OpenDB() → leaking mork stuff out of nsGlobalHistory::OpenDB()
Updated•24 years ago
|
Target Milestone: Future → mozilla0.9
Updated•24 years ago
|
Component: History: Session → History: Global
Comment 21•24 years ago
|
||
moving a few 0.9 bugs out to 1.0 to lighten my 0.9 load.
Target Milestone: mozilla0.9 → mozilla1.0
Comment 22•23 years ago
|
||
nav triage team: Marking nsbeta1+ and leaving at mozilla1.0
Comment 23•23 years ago
|
||
argh.. take two at reassigning history bugs to new owner
Assignee: alecf → blakeross
Status: ASSIGNED → NEW
Target Milestone: mozilla1.0 → ---
Assignee | ||
Comment 25•23 years ago
|
||
I think this is fixed now.
Status: NEW → RESOLVED
Closed: 23 years ago
QA Contact: claudius → stephend
Resolution: --- → FIXED
I'm assuming that Bruce's testcase refers only to the browser component, so this is indeed fixed. I ran Purify on Windows 2000 and mozilla.exe with no arguments, and then just shutdown after http://www.mozilla.org loaded. There isn't a single trace of Mork in my output. Verified FIXED. (Note, we still have Mork leaks elsewhere, especially mail/news).
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•