156940 - Crashes on close [@ morkRowObject::CloseRowObject] Trunk M11A M100

Status: VERIFIED FIXED

(SeaMonkey :: Autocomplete, defect)

Description

[greer]

Crashes with this signature are showing up in the Trunk (15 incidents), M100
(19) and the M11A release (51). User comments point to a crash at close of the
browser, following some active rummaging around in mail.

Looks like it is related to bug 104659 (V. Fixed) which was crasing at the same
signature.

Trunk Build ID range: 2002070108 to 2002070813
Stack Trace (Trunk):

	 morkRowObject::CloseRowObject
[c:/builds/seamonkey/mozilla/db/mork/src/morkRowObject.cpp  line 607]
	 morkRowObject::CloseMorkNode
[c:/builds/seamonkey/mozilla/db/mork/src/morkRowObject.cpp  line 87]
	 morkRowObject::~morkRowObject
[c:/builds/seamonkey/mozilla/db/mork/src/morkRowObject.cpp  line 95]
	 morkRowObject::`scalar deleting destructor'
	 morkObject::Release
[c:/builds/seamonkey/mozilla/db/mork/src/morkObject.cpp  line 68]
	 nsCOMPtr_base::~nsCOMPtr_base
[c:/builds/seamonkey/mozilla/xpcom/glue/nsCOMPtr.cpp  line 65]
	 nsAutoCompleteItem::`scalar deleting destructor'
	 nsAutoCompleteItem::Release
[c:/builds/seamonkey/mozilla/xpfe/components/autocomplete/src/nsAutoComplete.cpp
 line 48]
	 nsSupportsArray::Clear
[c:/builds/seamonkey/mozilla/xpcom/ds/nsSupportsArray.cpp  line 560]
	 nsSupportsArray::DeleteArray
[c:/builds/seamonkey/mozilla/xpcom/ds/nsSupportsArray.cpp  line 304]
	 nsSupportsArray::`vector deleting destructor'
	 nsCOMPtr_base::~nsCOMPtr_base
[c:/builds/seamonkey/mozilla/xpcom/glue/nsCOMPtr.cpp  line 65]
	 nsAutoCompleteResults::~nsAutoCompleteResults
[c:/builds/seamonkey/mozilla/xpfe/components/autocomplete/src/nsAutoComplete.cpp
 line 128]
	 nsAutoCompleteResults::`scalar deleting destructor'
	 nsAutoCompleteItem::Release
[c:/builds/seamonkey/mozilla/xpfe/components/autocomplete/src/nsAutoComplete.cpp
 line 48]
	 XPCJSRuntime::GCCallback
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp  line 539]
	 0x550004c2
 
 	Source File : c:/builds/seamonkey/mozilla/db/mork/src/morkRowObject.cpp line : 607

Comment 1

[greer]

cc'ing David B. who worked on the previous crash at
morkRowObject::CloseRowObject in bug 104659.

Comment 2

[David :Bienvenu]

this is a crash in the address book code, actually, or the auto complete code,
to be more precise. I'm going to reassign this to Cavin for now. My guess is
that the auto complete results are hanging around after the address book db has
been closed, for some reason, and then getting garbage collected. I don't know
how the ref-counting is working between the auto complete items and the addr db,
but it sounds like there may be a problem.

Comment 3

[Aleksander Adamowski]

Did those reports include TB8171938X ? It's mine, and I can say that as of build
2002071021 the issue is gone.

Comment 4

[greer]

Aleksander, TB8171938X was at "nsExceptionService::DoDropThread()". Different
issue. Thank you, though.

Comment 5

[Boris Zbarsky [:bzbarsky]]

*** Bug 179352 has been marked as a duplicate of this bug. ***

Comment 6

[Olivier Cahagne]

*** Bug 180413 has been marked as a duplicate of this bug. ***

Comment 7

[David :Bienvenu]

I'm sorry; this isn't the address book auto complete; it's the browser url bar
auto complete window. I'll take this for now, but it looks like auto complete is
getting cleaned up after the history db has been shutdown, and thus crashing.

Comment 8

[David :Bienvenu]

The problem is that js is holding onto the autocomplete results, and it looks
like gc is running after the history db has been shutdown. We need to clean up
the auto complete results before the history db gets shutdown.

NTDLL! 77fa018c()
nsDebug::Break(const char * 0x0bca0e10, int 0x00000039) line 298 + 25 bytes
nsDebug::Assertion(const char * 0x0bca2c34, const char * 0x0bca0e22, const char
* 0x0bca0e10, int 0x00000039) line 280 + 37 bytes
mork_assertion_signal(const char * 0x0bca2c34) line 57 + 96 bytes
morkRowObject::CloseRowObject(morkEnv * 0x1223c048) line 607 + 47 bytes
morkRowObject::CloseMorkNode(morkEnv * 0x1223c048) line 86 + 30 bytes
morkRowObject::~morkRowObject() line 94 + 43 bytes
morkRowObject::`scalar deleting destructor'(unsigned int 0x00000001) + 55 bytes
morkObject::Release(morkObject * const 0x0b17cd68) line 68 + 547 bytes
morkRowObject::Release(morkRowObject * const 0x0b17cd68) line 121 + 36 bytes
nsCOMPtr<nsISupports>::~nsCOMPtr<nsISupports>() line 733
nsAutoCompleteItem::~nsAutoCompleteItem() line 57 + 17 bytes
nsAutoCompleteItem::`scalar deleting destructor'(unsigned int 0x00000001) + 55 bytes
nsAutoCompleteItem::Release(nsAutoCompleteItem * const 0x10bef2f8) line 48 + 543
bytes
nsSupportsArray::Clear(nsSupportsArray * const 0x0b7403a0) line 560 + 179 bytes
nsSupportsArray::DeleteArray() line 305
nsSupportsArray::~nsSupportsArray() line 147 + 14 bytes
nsSupportsArray::`vector deleting destructor'(unsigned int 0x00000001) + 202 bytes
nsSupportsArray::Release(nsSupportsArray * const 0x0b7403a0) line 239 + 439 bytes
nsCOMPtr<nsISupportsArray>::~nsCOMPtr<nsISupportsArray>() line 491
nsAutoCompleteResults::~nsAutoCompleteResults() line 125 + 40 bytes
nsAutoCompleteResults::`scalar deleting destructor'(unsigned int 0x00000001) +
55 bytes
nsAutoCompleteResults::Release(nsAutoCompleteResults * const 0x0b740960) line
114 + 543 bytes
XPCJSRuntime::GCCallback(JSContext * 0x105423c0, JSGCStatus JSGC_END) line 546 +
49 bytes
DOMGCCallback(JSContext * 0x105423c0, JSGCStatus JSGC_END) line 1641 + 77 bytes
js_GC(JSContext * 0x105423c0, unsigned int 0x00000000) line 1399 + 52 bytes
js_ForceGC(JSContext * 0x105423c0, unsigned int 0x00000000) line 993 + 37 bytes
js_DestroyContext(JSContext * 0x105423c0, int 0x00000002) line 242 + 35 bytes
JS_DestroyContext(JSContext * 0x105423c0) line 895 + 35 bytes
nsJSContext::~nsJSContext() line 464 + 48 bytes
nsJSContext::`scalar deleting destructor'(unsigned int 0x00000001) + 55 bytes
nsJSContext::Release(nsJSContext * const 0x122994e8) line 487 + 546 bytes
nsTimerImpl::~nsTimerImpl() line 180 + 69 bytes
nsTimerImpl::`scalar deleting destructor'(unsigned int 0x00000001) + 55 bytes
nsTimerImpl::Release(nsTimerImpl * const 0x0a538028) line 94 + 92 bytes
nsTimerManager::~nsTimerManager() line 536 + 53 bytes
nsTimerManager::`scalar deleting destructor'(unsigned int 0x00000001) + 55 bytes
nsTimerManager::Release(nsTimerManager * const 0x04e69620) line 517 + 433 bytes
nsCOMPtr_base::assign_assuming_AddRef(nsISupports * 0x00000000) line 436
nsCOMPtr_base::assign_with_AddRef(nsISupports * 0x00000000) line 73 + 30 bytes
nsCOMPtr<nsISupports>::operator=(nsISupports * 0x00000000) line 795 + 30 bytes
FreeServiceContractIDEntryEnumerate(PLDHashTable * 0x0287d7a4, PLDHashEntryHdr *
0x04812f34, unsigned int 0x00000356, void * 0x00000000) line 1925 + 31 bytes
PL_DHashTableEnumerate(PLDHashTable * 0x0287d7a4, int (PLDHashTable *,
PLDHashEntryHdr *, unsigned int, void *)* 0x10115b70
FreeServiceContractIDEntryEnumerate(PLDHashTable *, PLDHashEntryHdr *, unsigned
int, void *), void * 0x00000000) line 603 + 108 bytes
nsComponentManagerImpl::FreeServices() line 1938 + 55 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 721 + 30 bytes
main(int 0x00000001, char * * 0x02876d48) line 1911 + 33 bytes

Comment 9

[Alec Flett]

ugh, sounds like we might need to do manual refcounting in the history
autocomplete results object? CC'ing dbradley for XPConnect advice here. How do
we ensure that JS releases an object "on time"?

Comment 10

[David :Bienvenu]

When I've had similar problems before, I've been told there's no way to force a
gc, so you have to clean up after yourself. What I did in mail might not work
for history, however, because I have wrapper objects around my mdb rows, and I
clean those up on shutdown. It looks like history is holding onto mdb rows in an
object held by the js code. Is there other code holding onto that object that
gets a shutdown notification and can cleanup the object so that it's empty?

Comment 11

[Alec Flett]

good point, a shutdown observer would be perfect, no need for manual refcounting!

if there isn't one already, I'm sure it wouldn't be hard to just write one up in
JS..

Comment 12

[Jay Patel [:jay]]

Making this a topcrash+ bug, since it is currently the #2 topcrash on the Trunk
and I have been personally seeing this a lot lately.  Here is my latest incident:

Stack Signature  morkRowObject::CloseRowObject 391987d8
Email Address [e-mail]
Product ID MozillaTrunk
Build ID 2002111408
Trigger Time 2002-11-16 17:16:19
Platform Win32
Operating System Windows NT 5.1 build 2600
Module mork.dll
URL visited nothing
User Comments again on closing my last open window
Trigger Reason Access violation
Source File Name c:/builds/seamonkey/mozilla/db/mork/src/morkRowObject.cpp
Trigger Line No. 607
Stack Trace
morkRowObject::CloseRowObject
[c:/builds/seamonkey/mozilla/db/mork/src/morkRowObject.cpp, line 607]
morkRowObject::CloseMorkNode
[c:/builds/seamonkey/mozilla/db/mork/src/morkRowObject.cpp, line 87]
morkRowObject::~morkRowObject
[c:/builds/seamonkey/mozilla/db/mork/src/morkRowObject.cpp, line 95]
morkRowObject::`scalar deleting destructor'
morkObject::Release [c:/builds/seamonkey/mozilla/db/mork/src/morkObject.cpp,
line 68]
nsCOMPtr_base::~nsCOMPtr_base
[c:/builds/seamonkey/mozilla/xpcom/glue/nsCOMPtr.cpp, line 65]
nsAutoCompleteItem::~nsAutoCompleteItem
[c:/builds/seamonkey/mozilla/xpfe/components/autocomplete/src/nsAutoComplete.cpp,
line 60]
nsAutoCompleteItem::`scalar deleting destructor'
nsAutoCompleteItem::Release
[c:/builds/seamonkey/mozilla/xpfe/components/autocomplete/src/nsAutoComplete.cpp,
line 114]
nsSupportsArray::Clear
[c:/builds/seamonkey/mozilla/xpcom/ds/nsSupportsArray.cpp, line 560]
nsSupportsArray::`vector deleting destructor'
nsCOMPtr_base::~nsCOMPtr_base
[c:/builds/seamonkey/mozilla/xpcom/glue/nsCOMPtr.cpp, line 65]
nsAutoCompleteResults::~nsAutoCompleteResults
[c:/builds/seamonkey/mozilla/xpfe/components/autocomplete/src/nsAutoComplete.cpp,
line 128]
nsAutoCompleteResults::`scalar deleting destructor'
nsAutoCompleteItem::Release
[c:/builds/seamonkey/mozilla/xpfe/components/autocomplete/src/nsAutoComplete.cpp,
line 114]
XPCJSRuntime::GCCallback
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp, line 539]
0x550004c2 

Comment 13

[Jay Patel [:jay]]

Actually, there was a spike in these crashes with builds from 11/14...and that
was the build I was using.  There haven't been a lot of crashes since that
day...did someone fix that problem on 11/14?

Comment 14

[David :Bienvenu]

It looks to me like this is all implemented in autoComplete.xml - I don't know
if it's possible to add a shutdown listener there or not. Is that the only
object that's holding onto the results? The other possibility is to not pass
mdrows around, but give the string for the uri instead of the mdb row - is there
any reason to keep the mdb row around?

Comment 15

[Alec Flett]

well, its just much more convenient to pass around the mdbrow... but actually
adding the shutdown listener shouldn't be too hard. I can't remember of XBL
objects can implement interfaces yet or not, but even if they can't we can have
some inner member object that is the observer.

Comment 16

[David :Bienvenu]

Alec, can you take this? It sounds like it would be easier for you to fix :-)

Comment 17

[Alec Flett]

Yeah, I suppose I'm a little more appropriate here.. *sigh* :)
cc'ing blake since he's technically the history owner...

Comment 18

[Matthias Versen [:Matti]]

*** Bug 180880 has been marked as a duplicate of this bug. ***

Comment 19

[Alec Flett]

I'm leaving for vacation.. since this is a topcrash+, I'm reassigning to blake..
are we still seeing this (I suspect we are)

Comment 20

[Jan Carpenter]

I'm not seeing this in talkback reports, and all the entries in the database
look fairly old. 

Comment 21

[David :Bienvenu]

this is a bug just waiting to reappear if we  don't do anything about it.

Comment 22

[Nick Konstadoglou]

well, I do get some crashes on close with 1.2. Not near as often as I used to
have with  other builds tho

Comment 23

[timeless]

ok. stephend and I hit this bug on the weekend. i hit it a bunch of times
(specifically i'm now stuck w/ german mozilla...).
tb15113630w was one of mine. it would appear that the steps i gave in the
comment are somewhat coincidental, but it doesn't matter.

Here's what should be done:
make addressbook listen for profile logout -- this event is *always sent*
(thanks to someone recently fixing the profile apis).
fwiw, profile bound processes such as autocomplete and history are not allowed
to be alive past this event. @see nsIProfileChangeStatus.idl for descriptions of
the events.

Comment 24

[Nicholas Allen]

*** Bug 185843 has been marked as a duplicate of this bug. ***

Comment 25

[David Bradley]

FWIW I can pretty much reproduce this at will. Just shutdown while a page is
loading.

Comment 26

[Matthias Versen [:Matti]]

*** Bug 185739 has been marked as a duplicate of this bug. ***

Comment 27

[Brant Gurganus]

I think this crash has been affecting me, but I have been unable to figure out
steps to reproduce it.

Talkback IDs:
TB15326619H
TB15301279M
TB15232430X
TB15113106X

Mozilla 2002121408 and I think a few previous builds have experienced this.

Attempts to reproduce that failed:
Start and exit Mozilla.
Start, make FTP connection, exit.
Start, attempt FTP download via plugin, exit.
Start mail, forward message to "spam" (which is address in address book) via
autocomplete, exit.
Start, load page, exit while loading.

Comment 28

[Simon Montagu :smontagu]

*** Bug 146508 has been marked as a duplicate of this bug. ***

Comment 29

[Simon Fraser [no longer active]]

I hit this *every* time I quit (Mac OS X CFM builds):

Thread 0 Crashed:
 #0   0x03fa5a7c in morkRowObject::CloseRowObject(morkEnv *)
 #1   0x03fa44bc in morkRowObject::CloseMorkNode(morkEnv *)
 #2   0x03fa4564 in morkRowObject::_dt(void)
 #3   0x03f9ac24 in morkObject::Release(void)
 #4   0x03fa4880 in morkRowObject::Release(void)
 #5   0x002fb764 in nsCOMPtr_base::_dt(void)
 #6   0x030bd238 in nsAutoCompleteItem::_dt(void)
 #7   0x030bcf64 in nsAutoCompleteItem::Release(void)
 #8   0x0025dca4 in nsSupportsArray::Clear(void)
 #9   0x0025cf30 in nsSupportsArray::DeleteArray(void)
 #10  0x0025c6c8 in nsSupportsArray::_dt(void)
 #11  0x0025cac4 in nsSupportsArray::Release(void)
 #12  0x002fb764 in nsCOMPtr_base::_dt(void)
 #13  0x030bdab0 in nsAutoCompleteResults::_dt(void)
 #14  0x030bd7b4 in nsAutoCompleteResults::Release(void)
 #15  0x02092bd4 in XPCJSRuntime::GCCallback(JSContext *, JSGCStatus)
 #16  0x028853ac in jsds_GCCallbackProc(JSContext *, JSGCStatus)
 #17  0x02cd5e70 in DOMGCCallback(JSContext *, JSGCStatus)
 #18  0x01f29ff0 in js_GC
 #19  0x01f29660 in js_ForceGC
 #20  0x01f06cb0 in JS_GC
 #21  0x02cf9c54 in nsDOMSOFactory::Observe(nsISupports *, char const *, wchar_t
const *)
 #22  0x00283a70 in nsObserverService::NotifyObservers(nsISupports *, char const
*, wchar_t const *)
 #23  0x002a6ef4 in NS_ShutdownXPCOM
 #24  0x001a5c30 in main

Comment 30

[Markus Hübner]

*** Bug 186261 has been marked as a duplicate of this bug. ***

Comment 31

[Scott Garfinkle]

I'm getting this crash several times a day using win32 1.3b 20021219. Sheesh.

Comment 32

[timeless]

Attachment: strong ref fix

Comment 33

[Matthias Versen [:Matti]]

*** Bug 186737 has been marked as a duplicate of this bug. ***

Comment 34

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 110118 [details] [diff] [review]
strong ref fix

>+        nsCOMPtr <nsIObserverService> os(do_GetService("@mozilla.org/observer-service;1"));
>+        if (!os)
>+            return NS_ERROR_FAILURE;

do_GetService will never fill in a null pointer without filling in its optional
rv, so if you want to be XPCOM-correct, this should be:

nsresult rv;
nsCOMPtr<nsIObserverService> os = do_GetService("...", &rv);
if (NS_FAILED(rv))
    return rv;

(which will (based on the looking at the code that I just did---and it's a
contract we want to keep) guarantee you a non-null |os| pointer).

Also note no space between nsCOMPtr and the template parameter, which is
consistent with the style here.

>+NS_IMETHODIMP nsAutoCompleteResults::Init()

|nsresult|, not NS_IMETHODIMP.	There's no need for this to be virtual.

>+    nsCOMPtr <nsIObserverService> os(do_GetService("@mozilla.org/observer-service;1"));
>+    if (!os)
>+        return NS_ERROR_FAILURE;

Same thing about do_GetService and about the space after nsCOMPtr.

>Index: autocomplete/src/nsAutoComplete.h

>+  NS_IMETHOD Init();

|nsresult| instead of |NS_IMETHOD| since there's no need for this to be
virtual.

I still need to read the bug and figure out why this fixes the crash...

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 110118 [details] [diff] [review]
strong ref fix

sr=dbaron if you:
 * revise per the comments above
 * Add a comment explaining that you're releasing the array to prevent holding
on to the results past when the history DB is closed, citing this bug number.

Comment 36

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

One other question, actually:  how often are these objects created and
destroyed?  Is it bad if they're all kept around forever?

Comment 37

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Also, how hard would it be to prevent the existence of one of these objects
from preventing the DB from being closed in the first place?)

Comment 38

[Matthias Versen [:Matti]]

*** Bug 186152 has been marked as a duplicate of this bug. ***

Comment 39

[timeless]

Attachment: weakref fix

> One other question, actually:  how often are these objects created and
> destroyed? 
every time you type a key. 
> Is it bad if they're all kept around forever?
yes. can i do anything about it? not easily.
i tried caching the objects, but to do that i need to make the cached objects
observers too, and it just gets too messy for now.

> how hard would it be to prevent the existence of one of these objects
> from preventing the DB from being closed in the first place?
you'd need to understand mork and autocomplete.

Comment 40

[timeless]

Comment on attachment 110229 [details] [diff] [review]
weakref fix

if you're satisfied and willing, please just check this in. i'm feeling sick
and it's unclear when i'll be around.

Comment 41

[Scott Garfinkle]

Is there any workaround? Is there some way for me to make sure this doesn't
happen? It's driving me crazy.

Comment 42

[Joe M]

A workaround is to disable autocomplete.

Comment 43

[Samir Gehani]

Nav triage team: nsbeta1+/adt1

Comment 44

[R.K.Aa.]

*** Bug 187137 has been marked as a duplicate of this bug. ***

Comment 45

[AMBROISE JEAN-MARIE]

Mozilla for MAC OS X Build 2002123003 continue to crashes on close 

Comment 46

[Owen Marshall (Not reading bugmail)]

*** Bug 186121 has been marked as a duplicate of this bug. ***

Comment 47

[Jo Hermans]

*** Bug 187346 has been marked as a duplicate of this bug. ***

Comment 48

[David Park]

Still seeing the crash when closing mozilla in lastest build.

Comment 49

[AMBROISE JEAN-MARIE]

Mozilla for MAC OS X build 2003010208 steel crash on close :-(

Comment 50

[Michael Lefevre]

Jean-Marie and others - we know that it still crashes, there is no need to keep
adding comments to say that.  nightly builds will continue to crash until a fix
is checked in.  as mentioned, a workaround is to go into preferences, navigator,
smart browsing, and disable autocomplete.

Comment 51

[Alec Flett]

Comment on attachment 110229 [details] [diff] [review]
weakref fix

I'm curious what the creation/destruction pattern of nsAutoCompleteResults is -
I mean, do we need to add EVERY object to the profile-before-change observer
list? Are there a lot of them? will this make every weak proxy object hang out
in the observer list until shutdown? 

Is there any way we can make the widget watch for shutdown and thus release the
results objects manually? 

even if we do have to do this for every result object, we should make sure that
~nsAutoCompleteResults() removes the observer so that those weak ref proxy
objects don't build up in the observer list.

Comment 52

[Matthias Versen [:Matti]]

*** Bug 187567 has been marked as a duplicate of this bug. ***

Comment 53

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 110229 [details] [diff] [review]
weakref fix

Like alecf said, you should remove the observer in ~nsAutoCompleteResults.

(What triggers the close of the history DB?  Is it a "profile-before-change"
notification?  If so, how do you guarantee the order?)

It still seems better if there were a way to keep the DB open (but nevertheless
have it flush to the disk when it is closed now).  This would avoid problems,
if, say, we started accessing the items within this array from JS as well...

Comment 54

[AMBROISE JEAN-MARIE]

For example: URL: http://www.toolinux.com/lininfo/news/ crashes Mozilla on close
each time with this URL.

Comment 55

[Michael Lefevre]

*** Bug 178688 has been marked as a duplicate of this bug. ***

Comment 56

[John Morrison]

*** Bug 187810 has been marked as a duplicate of this bug. ***

Comment 57

[David :Bienvenu]

the history db gets closed when the nsGLobalHistory object is released, and I
believe that happens when the global history service is released, so it's
dependent on the order of service shutdown. Autocomplete is keeping hold of mork
objects past this point, and that's really way too long.

Comment 58

[timeless]

bienvenu: the problem is that both autocomplete and globalhistory need to stop
holding references to profile files at profile logout. when autocomplete is
fixed to give up its things at logout, this problem goes away *temporarily*. the
question is will it come back the moment globalhistory is fixed to release its
database at profile logout.

My feeling is that global history needs to do its work at profile logout and
autocomplete will have to do its work at maybeprofilelogout.

Comment 59

[[e-mail] (gone - use [e-mail] instead)]

*** Bug 187682 has been marked as a duplicate of this bug. ***

Comment 60

[[e-mail] (gone - use [e-mail] instead)]

*** Bug 186323 has been marked as a duplicate of this bug. ***

Comment 61

[David :Bienvenu]

timeless, I was just pointing out that the history code is holding onto the db
pretty much for as long as it can, so dbaron's suggestion of making it hold onto
it longer than it does now does not seem workable to me.

I don't know if your approach will work - it sounds like it should. I'm
surprised the history code wasn't cleaning up after a profile logout already -
if you switch profiles, you need to release the current history db and
presumably that worked at some point when profile switching worked. 

the way the mailnews code handles all this is that if you're holding onto a db
reference, you need to register yourself as a db change listener, and you'll get
notified just before a db is closed out from under you - when you get such a
notification, you have to null out your references to any db objects, like
mdbrows. That approach should work here too. But for now, we just need to stop
the crashing.

Comment 62

[Alec Flett]

I think timeless has the right general approach, its just at too granular a
level - instead of making each result an observer, we should just make the whole
url bar autocomplete object (whatever that may be) an observer, and make it
release all its children.

Comment 63

[David :Bienvenu]

yes, I agree with Alec.

Comment 64

[Conrad Carlen (not reading bugmail)]

> I'm surprised the history code wasn't cleaning up after a profile logout
already - if you switch profiles, you need to release the current history db

The history code is closing and then opening the DB on a profile switch. I think
the problem is that, when this happens, there is still data left around
(autocomplete) which references the now closed DB, and so it crashes. Basically,
what David said in comment 8. I had this problem with profile switching and NSS
objects. What seems to have solved this is to force GC to happen after
"profile-change-teardown," the phase in which all windows are destroyed, and
before "profile-before-change", the phase in which global history shuts down its
DB. Otherwise, objects referenced by the JS context aren't released by GC until
a timer fires - long after the window is closed, which is too late.

The reason I bring up this business with profile shutdown is that, with bug
175320, the profile is now being shut down when we quit the app. Though this bug
predates that one, 175320 may have caused this to happen more frequently.  

Comment 65

[Owen Marshall (Not reading bugmail)]

*** Bug 188073 has been marked as a duplicate of this bug. ***

Comment 66

[Matthias Versen [:Matti]]

*** Bug 188134 has been marked as a duplicate of this bug. ***

Comment 67

[[e-mail] (gone - use [e-mail] instead)]

*** Bug 188372 has been marked as a duplicate of this bug. ***

Comment 68

[[e-mail] (gone - use [e-mail] instead)]

*** Bug 188191 has been marked as a duplicate of this bug. ***

Comment 69

[John Morrison]

*** Bug 186882 has been marked as a duplicate of this bug. ***

Comment 70

[Judson Valeski]

if anyone needs more stacks of this (internal only, sorry). checkout the dozen
or so talkback reports I've submitted over the past few weeks w/ this at the bottom.

http://climate.mcom.com/reports/veryfastsearchEmailNEW.cfm?email=valeski%40netscape.com&searchemail=simple

Comment 71

[Alec Flett]

Attachment: Manually clear the 'param' attribute on close

ok, I got tired of waiting on this and dove in myself. Turns out this is
incredibly easy! I had to change two things:
1) in the destructor for the XBL widget, call clearResults()
2) in clearResults() I set the 'param' to null

that's it. No ownership changes, no observers, etc.
a really straight forward fix.

Comment 72

[Alec Flett]

Comment on attachment 111437 [details] [diff] [review]
Manually clear the 'param' attribute on close

requesting reviews from hewitt and blake. Anyone else on the CC list is more
than welcome to do the reviews as well! The sooner we get this fix reviewed,
the sooner I'll fix this crash...!

Comment 73

[Simon Fraser [no longer active]]

Comment on attachment 111437 [details] [diff] [review]
Manually clear the 'param' attribute on close

May the gods be thanked

Comment 74

[Simon Fraser [no longer active]]

Comment on attachment 111437 [details] [diff] [review]
Manually clear the 'param' attribute on close

May the gods be thanked

Comment 75

[Alec Flett]

thanks all.. the fix is IN!

Comment 76

[[e-mail] (gone - use [e-mail] instead)]

Is this a wallpaper fix (IE, should the bug remain open for additional fixes),
or was this the finished, intended fix and should the bug be marked Resolved
FIXED?  Thanks for fixing!

Comment 77

[Alec Flett]

oops, this is the actual fix. I meant to mark this FIXED in my last comment!

Comment 78

[Bill Mason]

*** Bug 189000 has been marked as a duplicate of this bug. ***

Comment 79

[Michael Lefevre]

*** Bug 188707 has been marked as a duplicate of this bug. ***

Comment 80

[Bill Mason]

Verified in build 2003011405 PC/Win98.

Comment 81

[Owen Marshall (Not reading bugmail)]

*** Bug 188891 has been marked as a duplicate of this bug. ***

Comment 82

[sammy_c]

Verified fixed in build 2003011508, Windows XP

Comment 83

[Ketil Thorsen]

Verified fixed in Build ID 2003011506, Windows XP

Comment 84

[Jay Patel [:jay]]

Thank you Alex!  Talkback data no longer shows any crashes after 1/13
builds...I'm glad to see this topcrasher go away. ;-) 

Comment 85

[Jerry Baker]

Now, on XP, if you close Mozilla the process hangs around eating 100% CPU. See
bug 189521. I suspect this fix.

Comment 86

[Simon Fraser [no longer active]]

Bug 189466?

Comment 87

[Alec Flett]

No this bug is not involved in every bug that involves mozilla hanging or
crashing on exit. this one is fixed.. move along.

Comment 88

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

*** Bug 189544 has been marked as a duplicate of this bug. ***