142310 - Crash if quitting while downloading; trunk [@ nsHashtable::Exists]

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect, P1)

Description

[Nahor]

Reproducible: always

1. Start to download a file
2. Select "File | Exit"

Actual result: crash

Expedted result: Well, crashing does what I want (mozilla is dead) but I'm
expecting something more... gracefull :)

Comment 1

[Matthias Versen [:Matti]]

Build ID ?
Do you have a talkback ID from that crash ?

Comment 2

[Nahor]

I even have several. 
TB5943950Q
TB5940832Y
TB5940717G
TB5940695Z
TB5940684K

The first ID is for build "Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.0.0+) Gecko/20020504"
The four others are from a slightly older build (couple days ago or so)

Comment 3

[Nahor]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.0+) Gecko/20020506

If it's of any help, here is the talkback from today's build:
TB6007250G

Comment 4

[sairuh (rarely reading bugmail)]

has anyone experienced this using branch bits? i haven't (using
2002.05.07.08-1.0.0 comm on win2k).

and, just tried to repro this using 2002.05.07.08-trunk bits (win2k, comm), and
wasn't able to.

Nahor, is this still happening for you with *today's* build?

Incident ID 5943950
Stack Signature nsHashtable::Exists c8fb8acc
Product ID MozillaTrunk
Build ID 2002050408
Trigger Time 2002-05-04 19:58:05
Platform Win32
Operating System Windows NT 5.0 build 2195
Module xpcom.dll
Trigger Reason Access violation
Source File Name nsHashtable.cpp
Trigger Line No. 260

nsHashtable::Exists [nsHashtable.cpp, line 260]
nsDownloadManager::DownloadEnded [nsDownloadManager.cpp, line 165]
nsDownload::OnStateChange [nsDownloadManager.cpp, line 1070]
nsDownloadProxy::OnStateChange [./../download-manager/src\nsDownloadProxy.h,
line 157]
nsExternalAppHandler::OnStopRequest [nsExternalHelperAppService.cpp, line 1370]
nsDocumentOpenInfo::OnStopRequest [nsURILoader.cpp, line 255]
nsStreamListenerTee::OnStopRequest [nsStreamListenerTee.cpp, line 25]
nsHttpChannel::OnStopRequest [nsHttpChannel.cpp, line 2909]
nsOnStopRequestEvent::HandleEvent [nsRequestObserverProxy.cpp, line 213]
PL_HandleEvent [plevent.c, line 597]
PL_ProcessPendingEvents [plevent.c, line 530]
nsEventQueueImpl::ProcessPendingEvents [nsEventQueue.cpp, line 392]

Comment 5

[Nahor]

Yes, it still happens (build 2002050708), talkback id: TB6038752Z

Comment 6

[sairuh (rarely reading bugmail)]

anyone else able to repro this crasher?

Nahor, another question: is QuickLaunch on for you? (fwiw, i don't have QL on.)

Comment 7

[Nahor]

no, I don't have quicklaunch

Comment 8

[Nahor]

I did a fresh install on a completely reinstalled system (win2k SP2 with all
patchs) (different machine the usual).
1. I downloaded current nightly build (2002050708) and installed it. At the end
Mozilla starts with a completely new profile (since no previous mozilla nor
netscape).
2. went to www.mozilla.org
3. clicked on the "etc." link for the nightly build
4. clicked on the source tar.gz (any big file will do).
5. the download manager showed up.
6. clicked "File | Exit" in the browser window .
7. crashed (talkback ID TB6042512G)

I tried again but this time I closed the Download manager window before
quitting, and it still crashed (TB6042678E).

Comment 9

[Matthias Versen [:Matti]]

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

Comment 10

[Matthias Versen [:Matti]]

adding keywords etc from the dupe

Comment 11

[Nahor]

Changed to all from bug 142922

Comment 12

[selmer (gone)]

Blake, any comments?

Comment 13

[Peter Trudelle]

nsbeta1+/adt1-rtm per Nav triage team

Comment 14

[sairuh (rarely reading bugmail)]

aha, i was finally able to do repro this. it took a fresh profile, too, at least
for me.

tested using 2002.05.07 *trunk* commercial bits on win2k and linux rh7.2.

1. create a new profile (i've multiple ones, so made one via the profile mgr).
2. launch profile.
3. like Nahor in comment 8, i went to
http://ftp.mozilla.org/pub/mozilla/nightly/latest/ and clicked on a large file
there.
4. helper app dlg appeared, i chose "save this file to disk" and clicked OK.
5. chose location in resulting file picker dlg, clicked Save.
6. after dl mgr window appears, then quickly go to File > Exit in browser window.

results: crash. talkback came up, but the internal server doesn't seem to be
behaving at the moment (cannot do my usual email search). i believe a report was
generated.

i wonder if what needed here is a profile that has been created with a recent
build? the profiles which didn't crash for me had been created a week or more ago...

side note: i couldn't repro this on mac 10.1.4 --but one thing to note here is
that the Download Mgr doesn't appear automatically (expected). i opened up the
dl mgr window before step (3), but no crash. perhaps related to the dl mgr
automatically opening...? (that was bug 137440, fixed 4/28, trunk only.)

let me check this out with an older build.

Comment 15

[sairuh (rarely reading bugmail)]

i cannot repro this using a profile i had created and launched with
2002.04.27.08-trunk bits on win2k.

could this have been caused by the fix for bug 137440? has anyone been able to
repro with a build from *before* 4/28?

Comment 16

[Nahor]

My profle isn't new. It's an imported profile from Netscape. The profile goes
back at least to April 16, more likely April 8 if the file dates are of any
indication.

Comment 17

[sairuh (rarely reading bugmail)]

interesting... do you recall with what build you did the import, ie, was it with
a recent build or one from 16 or 8 april? just wondering...

side note: i couldn't repro this using today's branch (2002.05.08.08-1.0.0 comm)
build, again using a profile created with that build.

Comment 18

[Nahor]

From your question I guess you didn't understand. My _Mozilla_ profile has been
created on April 16 or 8 from a Netscape profile (which is more than 3 years
old, likely 6).
What I don't remember is if I imported it with RC1 then reused it when I
returned to a Trunk build or if I trashed my old Mozilla profile after RC1 and
reimported it from Netscape when I went back to Trunk.

side note: I'm going to try today's build then.

Comment 19

[Nahor]

Still crashing with my current profile (build 2002050807, Talkback: TB6085102Z)

Comment 20

[Nahor]

I also tried with a fresh import of my netscape profile (crash TB6085224G) and
with a completely new profile (crash TB6085433Y)
So trunk isn't fixed. I'm going to try with last-1.0.0 now.

Comment 21

[Nahor]

Well, it's working with the branch but it's not the download manager that is
used but the progress dialog and this was working before. 
How do you make the download manager the default? I have read about
"browser.downloadmanager.behavior" in prefs.js but it doesn't seems to work
(tried 0, 1 and 2). Is it trunk specific?

Comment 22

[Jan Carpenter]

This is #6 on todays trunk topcrash report with 122 crashes reported over the
last 10 days.  Here is todays info from the top 40:

 	Source File : nsHashtable.cpp line : 260

     Count   Offset    Real Signature
[ 83   nsHashtable::Exists c8fb8acc - nsHashtable::Exists ]
 
     Crash date range: 2002-04-29 to 2002-05-08
     Min/Max Seconds since last crash: 23 - 332295
     Min/Max Runtime: 23 - 754901
     Keyword List : download(12), load(12), crash(6),  
     Count   Platform List 
     50   Windows NT 5.0 build 2195
     26   Windows NT 5.1 build 2600
     4   Windows 98 4.10 build 67766446
     2   Windows NT 4.0 build 1381
     1   Windows 98 4.90 build 73010104
 
     Count   Build Id List 
     11   2002042908
     9   2002050708
     9   2002050108
     8   2002050408
     7   2002050508
     6   2002050807
     6   2002050208
     4   2002050604
     4   2002050308
     4   2002043010
     3   2002050608
     3   2002043003
     2   2002050705
     2   2002050104
     2   2002043008
     1   2002050304
     1   2002050204
     1   2002042903
 
     No of Unique Users        66
 
 Stack trace(Frame) 

	 nsHashtable::Exists
[nsHashtable.cpp  line 260] 
	 nsDownloadManager::DownloadEnded
[nsDownloadManager.cpp  line 166] 
	 nsDownload::OnStateChange
[nsDownloadManager.cpp  line 1086] 
	 nsDownloadProxy::OnStateChange
[./../download-manager/src\nsDownloadProxy.h  line 157] 
	 nsExternalAppHandler::OnStopRequest
[nsExternalHelperAppService.cpp  line 1369] 
	 nsDocumentOpenInfo::OnStopRequest
[nsURILoader.cpp  line 255] 
	 nsStreamListenerTee::OnStopRequest
[nsStreamListenerTee.cpp  line 25] 
	 nsHttpChannel::OnStopRequest
[nsHttpChannel.cpp  line 2909] 
	 nsOnStopRequestEvent::HandleEvent
[nsRequestObserverProxy.cpp  line 213] 
	 PL_HandleEvent
[plevent.c  line 597] 
	 PL_ProcessPendingEvents
[plevent.c  line 530] 
	 nsEventQueueImpl::ProcessPendingEvents
[nsEventQueue.cpp  line 392]  
 
     (6076483)	Comments: downloading openoffice 1.0.0
     (6058927)	Comments: I closed Mozilla by Ctrl+Q
     (6058847)	Comments: I closed Mozilla by Ctrl+Q
     (6058314)	Comments: trying to erase entries in download manager
     (6055255)	URL: www.tactical-ops.to
     (6055255)	Comments: I closed the browser  thanks for ruining my 100 meg download
     (6054872)	URL: www.tactical-ops.to
     (6054872)	Comments: Closing the browser
     (6038940)	URL: http://games.sohu.com/fightgame/fight3.swf
     (6038940)	Comments: Was opening http://games.sohu.com/fightgame/fight3.swf in a new
tab  closed the page  browser crashed.
     (6034882)	URL: http://ftp.mozilla.org/pub/mozilla/nightly/latest/
     (6034882)	Comments: downloading latest nightly build...download manager opened  and
moz. froze up like a deer in headlights.
     (6014170)	URL: http://www.n-tv.de
     (6014170)	Comments: Crash on exit via quicklaunch icon. Java Plug-In was running.
     (5995556)	URL: http://www.mut.de/media/buecher/autocadlt_komp/index.htm
     (5986245)	URL: http://download.com.com/3001-2379-8923437.html
     (5986245)	Comments: i wanted to download a trial of copernic. this site (mentioned)
appeared  the download manager opened but it didn't respond. when i tried to use
it/to activate the download it didn't react. when i closed the window if the
download manager it crashed
     (5985185)	Comments: i was surfing around the sites of www.google.de 
downloads.cnet.com (i don't know anymore the adresses of the sites i visited
before ;-( )when i wanted to download a software the downloadmanager didn't
react...when i closed it and mozilla --> the
     (5985185)	Comments:  crashedby the way: the download manager's great!but you should
improve  how it shows how many time's left for the download
     (5956971)	Comments: closed the quick launch in the tray
     (5922980)	URL: http://www.mozillazine.org/
     (5920332)	Comments: I closed all windows  then the download manager while it was
downloading a file :(
     (5919382)	Comments: Closing netscape after downloading soemthing
     (5904296)	URL:
http://ftp.mozilla.org/pub/mozilla/nightly/latest/mozilla-win32-talkback.zip
     (5863856)	URL: www.foodtv.com
     (5863856)	Comments: closing after a download had started in the download manager  but
i closed that window first
     (5844246)	URL: http://www.tdwaterhouse.com
     (5844246)	Comments: crash on exit
     (5806497)	Comments: closing latest window while hidden download manager was active
(getting some file from net).
     (5782302)	Comments: I shut down mozilla. Download manager was still running. I closed
the d/l manager and it crashed. Apps running: MSN mesg  Pocomail  tomcat on WinXP
     (5770645)	Comments: Quitting a download of a milestone build.  Had downloaded
Moz1.0.0+ but mail hangs. (WinXPH  on starting mail  processor time goes to 100%
and stays there for 30+sec on P3 1GHz 513M)  Started download  realized logged
in as user & need to go to root so
     (5770645)	Comments:  cancelled it.
 
[ 18   nsHashtable::Exists 6eace8d9 - nsHashtable::Exists ]
     8   Windows NT 5.0 build 2195
     7   Windows NT 5.1 build 2600
     2   Windows NT 4.0 build 1381
     1   Windows 98 4.10 build 67766222
     3   2002050208
     2   2002050708
     2   2002050608
     2   2002050508
     2   2002050408
     2   2002050108
     2   2002042908
     1   2002050308
     1   2002050104
     1   2002043008
     (6046928)	URL:
ftp://ftp.edgefiles.com/edgegaming.com/swarm/KingAlbrecht/wizwars_v105_full.zip
     (6046928)	Comments: Same crash. at nsHashtable::Exists
     (6046679)	URL:
ftp://ftp.edgefiles.com/edgegaming.com/swarm/KingAlbrecht/wizwars_v105_full.zip
     (6046679)	Comments: Copied link into browesr  selected "Save to disk"  closed all
browser windows during download (including download mgr.) -> Crash.
     (6011509)	Comments: I was shutting down Mozilla RC1 and it just crashed on me.
     (6011420)	URL: news.chinatimes.com.tw
     (6011420)	Comments: Shutting down browser. Mail client was also killed at the same
time and the aget apeared.
     (5952757)	Comments: closing moz
     (5834842)	URL:
ftp://ftp.mozilla.org/pub/mozilla/nightly/2002-05-01-08-trunk/mozilla-win32-installer.exe
     (5834842)	Comments: I was downloading the lastest build of Mozilla 1.0RC1 and closed
down Mozilla before the download was complete. Then Mozilla crashed.
 
[ 6   nsHashtable::Exists e16be92f - nsHashtable::Exists ]
     Count   Platform List 
     2   Windows NT 5.1 build 2600
     2   Windows NT 5.0 build 2195
     2   Windows 98 4.10 build 67766446
 
     Count   Build Id List 
     2   2002050408
     2   2002050108
     1   2002050308
     1   2002050304
     (6076652)	URL: http://www.kaspersky.ee/
     (6041356)	URL: ftp.redhat.com
     (6041356)	Comments: Cancelling a sownload of a very large file from a very slow ftp
connection  after about an hour of waiting.
     (5947946)	URL:
ftp://ftp.edgefiles.com/edgegaming.com/swarm/KingAlbrecht/wizwars_v105_full.zip
     (5947946)	Comments: Download Manager was downloading the file  and I closed the
associated Mozilla Browser window. The Download Manager then crashed.

[ 2   nsHashtable::Exists() e6c17603 - nsHashtable::Exists() ]
     2   Linux 2.4.18-0.13   
     Count   Build Id List 
     2   2002050210
 
[ 2   nsHashtable::Exists() 3898052d - nsHashtable::Exists() ]
[ 1   nsHashtable::Exists() b879754b - nsHashtable::Exists() ]
[ 1   nsHashtable::Exists() 99ba2447 - nsHashtable::Exists() ]
[ 1   nsHashtable::Exists() 87a5a8dc - nsHashtable::Exists() ]
[ 1   nsHashtable::Exists() 8292293f - nsHashtable::Exists() ]
[ 1   nsHashtable::Exists() 6697a0e8 - nsHashtable::Exists() ]

     5   Linux 2.4.18-6mdk   
     1   Linux 2.4.8-26mdk   
     1   Linux 2.4.7-10smp   
 
     Count   Build Id List 
     2   2002050307
     1   2002050710
     1   2002050610
     1   2002050310
     1   2002050210
     1   2002050110
 

Comment 23

[sairuh (rarely reading bugmail)]

yeah, this looks trunk-only.

Nahor, info on the pref that controls this dl mgr behavior is:

    browser.downloadmanager.behavior

its values are:

0 - open the download mgr window
1 - open a progress dlg
2 - do nothing

Comment 24

[Blake Ross]

That pref won't work on the branch.  However, whether or not download manager is
on by default should not impact this.  The download manager tracks the download
either way, so we go through the same codepath.  I suspect that something
unrelated to download manager is causing this crash, but I'll have to look into
it further.  In the mean time, I'm not sure I understand why this is
nsbeta1+/adt1-rtm if it's trunk-only.

Comment 25

[Blake Ross]

Sarah, can you help me narrow down exactly when this regressed?

Comment 26

[sairuh (rarely reading bugmail)]

narrowed it down to this period, using linux commercial trunk bits:

2002.04.28.07-trunk - works fine, no crash when quitting during download
2002.04.28.21-trunk - broken, crashes when quitting during download

in the 7am build, the download manager didn't automatically appear; instead the
regular progress dlg came up. in the 9pm build, the download manager
automatically appeared for the download. (side note: for both builds i created
new profiles.)

Comment 27

[Nahor]

Even on yesterday's build (2002050908 Win2k), if you change the default to
progress dialog (browser.downloadmanager.behavior=1) which doesn't involve the
download manager, it doesn't crash. For the behaviors 0 and 2 which use the
manager, it crashes.

Comment 28

[Peter Trudelle]

Blake & Sairuh: Is this really only a problem on the trunk?  If so, we need to
get it off the MachV radar.

Comment 29

[Blake Ross]

This will occur on the branch once the pref goes on the branch.

Cc'ing cvs blamers -- any idea how nsHashtable::Exists() could be crashing?  I
need to get a debug build to investigate further.

Comment 30

[Blake Ross]

Hm, judging from the (slightly off) stack line numbers, I guess it's crashing on

PLHashNumber hash = aKey->HashCode();

so the key I'm passing in must be null or garbage when we're shutting down. Need
to figure out why.

Comment 31

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

It could also be crashing if |this| is null or garbage, since it's non-virtual.

The disassembly and registers would help to show what the cause is.  (Go to a
talkback report, with a URL like
http://cyclone/reports/incidenttemplate.cfm?bbid=16077289 , click on the "Code
around the PC" tab, and get the register info and the disassembly (where the
first line of the disassembly should be red -- if it's a different line, need to
note that).

Comment 32

[Blake Ross]

x86 Registers:
EAX: 0905ed4e EBX: 0905ed4e ECX: 0012fd34 EDX: 0012fd34
ESI: 00000000 EDI: 0012fd2c ESP: 0012fd14 EBP: 0012fd3c
EIP: 60e38619 cf pf af zf sf of IF df nt RF vm   IOPL: 0
CS: 001b DS: 0023 SS: 0023 ES: 0023 FS: 0038 GS: 0000

Code Around the PC: 60e38619 8b4604           mov     eax,[esi+0x4]
60e3861c 85c0             test    eax,eax
60e3861e 7408             jz      60e38628
60e38620 50               push    eax
60e38621 ff15d8b1e760     call    dword ptr [60e7b1d8]
60e38627 59               pop     ecx
60e38628 837e2800         cmp     dword ptr [esi+0x28],0x0
60e3862c 8d4608           lea     eax,[esi+0x8]
60e3862f 57               push    edi
60e38630 53               push    ebx
60e38631 50               push    eax
60e38632 7407             jz      60e3863b
60e38634 e849210400       call    60e7a782

I have discovered a bit more about this crash, I'll post it soon.

Comment 33

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

Comment 32 shows that the crash occurs because |this| is null in
nsHashtable::Exists, so we crash when trying to look at |mLock|.

Comment 34

[Peter Trudelle]

That pref must not go onto the branch until this is fixed.

Comment 35

[Blake Ross]

dbaron is correct, |this| is null.  Actually, the whole download manager service
has already been shut down by the time the nsIDownload receives notification
that the transfer has stopped and calls various nsIDownloadManager functions. 
The only reason this doesn't occur when using the progress dialogs instead of
download manager is because the progress dlg holds a ref to the download manager
as its observer... 

Comment 36

[Suresh Duddi (gone)]

You mean we dont really abort the download, kill all datastructures/callbacks
before going into shutdown. That sounds bad (with and without download manager).

Comment 37

[Blake Ross]

Right, listeners are still getting progress notifications after services have
been shut down:

nsDownload::OnStateChange(nsDownload * const 0x0334bd8c, nsIWebProgress *
0x00000000, nsIRequest * 0x032b5298, unsigned int 16, unsigned int 0) line 1062
+ 35 bytes
nsDownloadProxy::OnStateChange(nsDownloadProxy * const 0x034821bc,
nsIWebProgress * 0x00000000, nsIRequest * 0x032b5298, unsigned int 16, unsigned
int 0) line 142 + 39 bytes
nsExternalAppHandler::OnStopRequest(nsExternalAppHandler * const 0x032880b0,
nsIRequest * 0x032b5298, nsISupports * 0x00000000, unsigned int 2152398850) line
1369
nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x0351c978,
nsIRequest * 0x032b5298, nsISupports * 0x00000000, unsigned int 2152398850) line 256
nsStreamListenerTee::OnStopRequest(nsStreamListenerTee * const 0x03675dd0,
nsIRequest * 0x032b5298, nsISupports * 0x00000000, unsigned int 2152398850) line 66
nsHttpChannel::OnStopRequest(nsHttpChannel * const 0x032b529c, nsIRequest *
0x035b9924, nsISupports * 0x00000000, unsigned int 2152398850) line 2897
nsOnStopRequestEvent::HandleEvent() line 213
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x02a84bf4) line 116
PL_HandleEvent(PLEvent * 0x02a84bf4) line 596 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00c5dd28) line 526 + 9 bytes
nsEventQueueImpl::ProcessPendingEvents(nsEventQueueImpl * const 0x00c4a778) line
388 + 12 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 563
main(int 1, char * * 0x004448a0) line 1814 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e7eb69()

In light of that, this probably belongs to someone in networking. But I will
keep digging.

Comment 38

[Blake Ross]

Passing to networking for a look.

Comment 39

[Darin Fisher]

necko must fire OnStopRequest... that's required.

-> docshell

Comment 40

[Suresh Duddi (gone)]

I think the download manager or download progress should cancel the download
before letting shutdown happen. Rather I wonder if shutdown should issue
something equivalent of cancel all network activity. Anyway it all seems like it
should happen from the client.

Comment 41

[Blake Ross]

The download manager cannot stop the download, it doesn't have that kind of
control: the downloader could be anyone (AIM, etc.).  It just listens to
notifications.

Comment 42

[Johnny Stenback (:jst)]

Hmm, the status whiteboard in this bug says this is trunk only, if that's true,
then why is this marked nsbeta1+?

Comment 43

[Blake Ross]

Because this can occur on the branch if you set the pref to have download
manager appear automatically (which will be possible once nsbeta1+ bug 137440 is
fixed on the branch).

Comment 44

[selmer (gone)]

http://bugzilla.mozilla.org/show_bug.cgi?id=137440 is marked adt1.0.0-, so it's
never going to get fixed on the branch.  In order to change that, you'll need to
raise this to the ADT's attention again by adding an adt1.0.1 keyword.  I'm not
sure why they would change stance on that bug at this point though...

Comment 45

[sairuh (rarely reading bugmail)]

true, but now that the Downloads pref panel is back on the branch, if the user
turns on the "open the dl mgr" option, s/he would encounter this.

Comment 46

[Blake Ross]

I was just looking into fixing this by cancelling all transferring downloads in
nsDownloadManager's dtor, but that's not possible right now because the download
manager would need to use rdf to get a list of the ongoing downloads, and by
then the rdf service has been shut down.  This demonstrates why networking
either needs to cancel the download itself, or, as Suresh said in comment 40,
networking should notify us that network activity is ending, and then dl mgr
could cancel.

Comment 47

[scottputterman]

adam, what's the ETA on this bug.  Blake, should this be reassigned to you?

Comment 48

[Blake Ross]

No, this is not something I can fix from my end.  Any object that listens to
notifications and then does something using a service, and receives the
notification upon quitting, can potentially run into this.  I will see if I can
somehow work around it.  Alternatively, we can remove the option to open the
download manager for downloads if we don't think this can get fixed by MachV.

Comment 49

[Adam Lock]

A trunk build from yesterday doesn't crash for me but the 1.0.1 branch does. 

Would it be possible to add code to the download manager listen to the
"quit-application" notification sent out by the goQuitApplication() routine and
have the download manager cancel all downloads at that point?

http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/globalOverlay.js#1

Download manager may even be able to pose a "You are downloading files, are you
sure you want to quit?" message and throw an exception to abort the quit.

Comment 50

[Adam Lock]

Attachment: Rouch patch

This illustrates my previous suggestion - add a "quit-application" observer to
the download manager and cancel downloads in progress when it arrives. It works
*most* of the time but I have observed a crash that I was unable to debug,
which might have been network events arriving after xpcom shutdown.

Personally I wonder if the download manager is ready for 1.0.1, since it
appears to suffer from some odd behaviour such as opening one download manager
window per download plus plenty of assertions in debug mode. Perhaps the pref
panel should be removed.

Comment 51

[Blake Ross]

Adam, the opening one window per download thing is fixed on the trunk...are you
saying it's not on the branch? That would be bad.

The assertions have to do with trees that have embedded progressmeters, and
isn't specific to download manager.

Comment 52

[Blake Ross]

Good catch!  The fix for bug 131762 hasn't been checked in on the branch.  I've
nominated it.

Any bug on the assertions should be filed to Jan Varga.

Comment 53

[Blake Ross]

Comment on attachment 87522 [details] [diff] [review]
Rouch patch

This is a good suggestion.  Not done quite correctly, since we can just use the
dom to enumerate the downloads (no need to select them all, etc.).  Also, the
bug to add a warning about canceled downloads being was minused long ago. I'll
produce an updated patch.

Comment 54

[Adam Lock]

Reassign to Blake

Comment 55

[Jaime Rodriguez, Jr.]

sairuh - shiould this still say [trunk only] in the status whiteboard?

Comment 56

[Blake Ross]

Attachment: new patch

Comment 57

[Blake Ross]

Attachment: better patch

Comment 58

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 88528 [details] [diff] [review]
better patch

sr=[e-mail]

Comment 59

[Brian Ryner (not reading)]

Comment on attachment 88528 [details] [diff] [review]
better patch

r=bryner

Comment 60

[Blake Ross]

Fixed on trunk.

Comment 61

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on adt's behalf) approval for checkin to the 1.0 branch, pending
drivers' approval. pls checkin this in asap, the add the "fixed1.0.1" keyword.

Comment 62

[Blake Ross]

Still plan on checking this in today, just waiting for branch opening...

Comment 63

[sairuh (rarely reading bugmail)]

this looks good on the trunk, using 2002.06.21.10 comm trunk bits on linux rh7.2.

Nahor, is this working for you now with the latest trunk build?

Comment 64

[Nahor]

Yes, it seems to work fine. I just tried a couple times on two different machine
and it didn't crash.

Comment 65

[sairuh (rarely reading bugmail)]

thanks for checking, Nahor!

marking verified [on the TRUNK].

Comment 66

[Asa Dotzler [:asa]]

Comment on attachment 88528 [details] [diff] [review]
better patch

a=asa (on behalf of drivers) for checkin to the branch.

Comment 67

[Asa Dotzler [:asa]]

approved for checkin to the branch. when you've landed this please change the
mozilla1.0.1+ keyword to fixed1.0.1. 

Comment 68

[Blake Ross]

Fixed on branch.

Comment 69

[sairuh (rarely reading bugmail)]

verified on the branch using 2002.06.27.08 comm bits on win2k.

Comment 70

[Jan Carpenter]

Reopening as a topcrash on the trunk.

Rank    StackSignature    Count  
15   nsHashtable::Exists   15
Source File : c:/builds/seamonkey/mozilla/xpcom/ds/nsHashtable.cpp line : 261

  Count   Offset    Real Signature
[ 12   nsHashtable::Exists f338067b - nsHashtable::Exists ]
 
     Crash date range: 2002-07-07 to 2002-07-15
     Min/Max Seconds since last crash: 427 - 177884
     Min/Max Runtime: 427 - 177884
     Keyword List :  
     Count   Platform List 
     5   Windows NT 5.0 build 2195
     3   Windows NT 5.1 build 2600
     2   Windows 98 4.10 build 67766446
     1   Windows NT 4.0 build 1381
     1   Windows 95 4.0 build 67109975
 
     Count   Build Id List 
     3   2002071008
     3   2002070908
     1   2002071204
     1   2002071004
     1   2002070808
     1   2002070708
     1   2002070608
     1   2002070508
 
     No of Unique Users        12
 
 Stack trace(Frame) 

	 nsHashtable::Exists	[c:/builds/seamonkey/mozilla/xpcom/ds/nsHashtable.cpp 
line 261] 
	 nsDownloadManager::DownloadEnded
[c:/builds/seamonkey/mozilla/xpfe/components/download-manager/src/nsDownloadManager.cpp
 line 174] 
	 nsDownload::OnStateChange
[c:/builds/seamonkey/mozilla/xpfe/components/download-manager/src/nsDownloadManager.cpp
 line 1188] 
	 nsWebBrowserPersist::OnStopRequest
[c:/builds/seamonkey/mozilla/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp
 line 676] 
	 nsStreamListenerTee::OnStopRequest
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsStreamListenerTee.cpp  line 66] 
	 nsHttpChannel::OnStopRequest
[c:/builds/seamonkey/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp  line
2929] 
	 nsOnStopRequestEvent::HandleEvent
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsRequestObserverProxy.cpp  line 213] 
	 PL_HandleEvent	[c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c  line 597] 
	 PL_ProcessPendingEvents	[c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c 
line 530] 
	 nsEventQueueImpl::ProcessPendingEvents
[c:/builds/seamonkey/mozilla/xpcom/threads/nsEventQueue.cpp  line 392]  
 
     (8344847)	Comments: There were 3 Downloads running in the backgroud
(downloadmanager-window closed) when I shut down mozilla. It crashed
     (8326572)	URL: www.theregister.co.uk
     (8258875)	Comments: crash after installling type ahead search XPI
     (8230340)	Comments: eating tofu
     (8222151)	Comments: Crashed on automatic restart by Quick Launch
     (8298242)	Comments: Closing the browser

Comment 71

[Blake Ross]

Something must have changed on the trunk.  I haven't touched download manager on
awhile.  Removing adt markings since it's trunk-only.

Comment 72

[Samir Gehani]

Nav triage team: nsbeta1+/adt1

Comment 73

[Alec Flett]

Attachment: make mCurrDownloads a non-pointer member

Ok, so my theory is that some screwy download path is never creating the
hashtable. Instead of putting a whole bunch of if (!mCurrDownloads)
mCurrDownloads = new nsHashtable() all over the place, I decided to make it
just a normal member variable, so the hashtable will always exist - this will
save is one extra allocation anyway, and avoid future codepaths where
mCurrDownloads doesn't exist.

Comment 74

[Jay Patel [:jay]]

this could be our 600th topcrash fix!  let's get this on the Trunk so we can
moniter Talkback data to see if the crashes go away.

just to clarify, this crash appears to be Trunk only (there haven't been any
incidents with the Gecko1.0 Branch).  were the nsbeta1+ and adt1 markings put in
on 7/19 for Buffy?

Comment 75

[Alec Flett]

yup, that's why I attached the patch.. (to get the 600th!) 
I just need reviews.. we don't even need approvals anymore..

I'll just take this and go bug blake for reviews...

Comment 76

[jag (Peter Annema)]

Comment on attachment 94027 [details] [diff] [review]
make mCurrDownloads a non-pointer member

sr=jag

Comment 77

[Darin Fisher]

Comment on attachment 94027 [details] [diff] [review]
make mCurrDownloads a non-pointer member

r=darin

Comment 78

[Alec Flett]

hey chofmann, I haven't checked this in just yet - I'm waiting desperately for
an open tree :)

Comment 79

[Jay Patel [:jay]]

it looks like bug 90571 was the one that took us to 600 earlier today...but that
wasn't even really a fix (a new version of the jre fixed that crash).  so as
soon as the tree opens you can officially take us to 601 alec!

Comment 80

[Alec Flett]

ok, NOW this is fixed :)

Comment 81

[sairuh (rarely reading bugmail)]

note: chris petersen and i cannot verify this using trunk builds on Mac OS X,
either with pre- or post-fix trunk builds. perhaps this is related to bug
155426. i guess we'll know for OS X if/when this get checked in on the branch.

anyhow, gonna get trunk bits for the other platforms to test this...

Comment 82

[sairuh (rarely reading bugmail)]

vrfy'd fixed on linux [2002.08.07.08] and win2k [2002.08.07.11] comm trunk builds.

unable to verify on mac os 9.x [2002.08.07.08 comm trunk], prolly due to bug
132027...

do ppl want this for the 1.0.1 branch?

Comment 83

[scottputterman]

removing adt1.0.1 based on comments that say this is trunk only.

Comment 84

[Michael Dunn]

Over to depstein for verification

Comment 85

[David Epstein]

Verified with commercial trunk Netscape 1.3b Mozilla/5.0 (Windows; U; Windows NT
5.1; en-US; rv:1.3b) Gecko/20030109 Netscape/7.01+ . No crashing. Download
manager remains even after quitting app, document successfully downloads.