leaking mork stuff out of nsGlobalHistory::OpenDB()

VERIFIED FIXED

Status

P3
normal
VERIFIED FIXED
19 years ago
2 months ago

People

(Reporter: bruce, Assigned: Bienvenu)

Tracking

({memory-leak})

Trunk
memory-leak

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

19 years ago
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]
Global history to waterson
Assignee: radha → waterson

Comment 2

19 years ago
david[b|mc]: is this the "mork was designed to leak" problem?
(Assignee)

Comment 3

19 years ago
I don't think files are built for leakage. but I could be wrong.

Comment 4

19 years ago
bruce: is nsGlobalHistory leaking? could you check the bloat logs? thanks.

Updated

19 years ago
Keywords: mlk
Summary: MLK: mork-stuff out of nsGlobalHistory::OpenDB() → mork-stuff out of nsGlobalHistory::OpenDB()
(Reporter)

Comment 5

19 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

19 years ago
XPCOM_MEM_LEAK_LOG says that nope, we aren't leaking nsGlobalHistory during 
mozilla-bin startup/shutdown.

Comment 7

19 years ago
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

19 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?

Comment 9

19 years ago
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

19 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

19 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

19 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

19 years ago
bruce: how are we doing on this one? should i reassign to you?
(Reporter)

Comment 14

19 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

19 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

19 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
goes back to waterson.
Assignee: radha → waterson

Updated

19 years ago
Status: NEW → ASSIGNED

Comment 18

19 years ago
-> rjc
Assignee: waterson → rjc
Status: ASSIGNED → NEW

Updated

18 years ago
Target Milestone: M18 → Future

Comment 19

18 years ago
Taking these history and history-related bugs.
Assignee: rjc → alecf

Updated

18 years ago
Status: NEW → ASSIGNED
Summary: mork-stuff out of nsGlobalHistory::OpenDB() → leaking mork stuff out of nsGlobalHistory::OpenDB()

Updated

18 years ago
Target Milestone: Future → mozilla0.9
nav triage team: not a beta stopper
Keywords: nsbeta1-

Updated

18 years ago
Component: History: Session → History: Global

Comment 21

18 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

17 years ago
nav triage team:

Marking nsbeta1+ and leaving at mozilla1.0
Keywords: nsbeta1- → nsbeta1+

Updated

17 years ago
Blocks: 92580

Comment 23

17 years ago
argh.. take two at reassigning history bugs to new owner
Assignee: alecf → blakeross
Status: ASSIGNED → NEW
Target Milestone: mozilla1.0 → ---
(Assignee)

Comment 24

17 years ago
taking.
Assignee: blakeross → bienvenu
(Assignee)

Comment 25

17 years ago
I think this is fixed now.
Status: NEW → RESOLVED
Last Resolved: 17 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

17 years ago
No longer blocks: 92580

Updated

2 months ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.