Last Comment Bug 683959 - crash [@ nsFtpState::R_retr() ]
: crash [@ nsFtpState::R_retr() ]
Status: RESOLVED FIXED
[mentor=jdm][lang=c++]
: crash
Product: Core
Classification: Components
Component: Networking: FTP (show other bugs)
: Trunk
: x86_64 Windows 7
: -- critical (vote)
: mozilla19
Assigned To: Srinath N [:srinath]
:
: Patrick McManus [:mcmanus]
Mentors:
http://www.gnupg.org/download/index.e...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-01 10:48 PDT by IU
Modified: 2012-11-17 05:13 PST (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stack trace (93.33 KB, text/plain)
2011-09-01 10:48 PDT, IU
no flags Details
Patch for bug 683959 - Adding null checks for mDataStream (2.41 KB, patch)
2012-11-14 18:10 PST, Srinath N [:srinath]
jduell.mcbugs: review+
Details | Diff | Splinter Review

Description IU 2011-09-01 10:48:16 PDT
Created attachment 557569 [details]
stack trace

When clicking the second FTP link for GnuPG 2.0, at http://www.gnupg.org/download/index.en.html, I got this crash.

The first crash happened immediately, the first time I clicked the second link (after clicking the first link and canceling), but subsequent crashes takes many tries before I'm able to get it to crash again.  To get it to crash, I basically clicked the FTP link.  If it does not crash, I cancel the save dialog and keep trying.

Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsFtpState::R_retr 	netwerk/protocol/ftp/nsFtpConnectionThread.cpp:1202
1 	xul.dll 	nsFtpState::Process 	netwerk/protocol/ftp/nsFtpConnectionThread.cpp:593
2 	xul.dll 	nsFtpState::OnControlDataAvailable 	netwerk/protocol/ftp/nsFtpConnectionThread.cpp:223
3 	xul.dll 	nsFtpControlConnection::OnInputStreamReady 	netwerk/protocol/ftp/nsFtpControlConnection.cpp:93
4 	xul.dll 	nsInputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:114
5 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:631
6 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:110
7 	xul.dll 	xul.dll@0xbbbac3 	
8 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:201
9 	xul.dll 	_SEH_epilog4 	
10 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:175
11 	xul.dll 	nsThreadManager::GetCurrentThread 	xpcom/threads/nsThreadManager.cpp:218
12 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:189
13 	xul.dll 	xul.dll@0xbbbac3 	
14 	xul.dll 	nsAppShell::Run 	widget/src/windows/nsAppShell.cpp:261
Comment 2 Hari R 2011-09-14 12:11:34 PDT
Unable to reproduce even after clicking tens of times in an x86_64 Linux 3.0 SMP environment.

I do not have a Windows dev env. Can someone who does please try and reproduce this?
Comment 3 quazgar 2012-04-30 08:45:25 PDT
I can confirm this with FF 12.0 on WinXP in a VirtualBox environment (Linux as host system). Tryong to open ftp://ftp.gnupg.org/gcrypt/gnupg/gnupg-2.0.19.tar.bz2.sig crashes Firefox there.
Comment 4 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-04-30 09:57:05 PDT
This might just need a null check for mDataStream, since every other use of it has one.
Comment 5 quazgar 2012-05-01 00:36:11 PDT
(In reply to daniel.hornung from comment #3)
> I can confirm this with FF 12.0 on WinXP in a VirtualBox environment (Linux
> as host system).

I have to retract this comment, since the problem in my case seems to be with VirtualBox: https://www.virtualbox.org/ticket/4478
Comment 6 IU 2012-05-01 07:22:51 PDT
(In reply to daniel.hornung from comment #5)
> (In reply to daniel.hornung from comment #3)
> I have to retract this comment, since the problem in my case seems to be
> with VirtualBox: https://www.virtualbox.org/ticket/4478

You seem to be conflating Firefox with a VM guest, for some reason.  Is it Firefox that crashed or Windows?  If Firefox, you should get the same crash signature in this summary, otherwise you're experiencing a different issue.
Comment 7 quazgar 2012-05-02 09:33:58 PDT
(In reply to IU from comment #6)Is it
> Firefox that crashed or Windows?  If Firefox, you should get the same crash
> signature in this summary, otherwise you're experiencing a different issue.

The VirtualBox environment crashes, I just thought it was improper handling of FF crashing plus improper handling of this of both Win and VB, but it seems it was only/mainly VB in this case, since the error was not limited to FF. So, it's a different issue and not FF's fault.
Comment 8 Wayne Mery (:wsmwk, NI for questions) 2012-08-06 04:57:56 PDT
UI, can you still reproduce this?

quite rare - only 3 version 14 crashes in past month. for example: bp-4cfd052b-ccd5-41f4-bbc7-725d42120723
Comment 9 IU 2012-08-07 02:13:45 PDT
(In reply to Wayne Mery (:wsmwk) from comment #8)
> UI, can you still reproduce this?
> 
> quite rare - only 3 version 14 crashes in past month. for example:
> bp-4cfd052b-ccd5-41f4-bbc7-725d42120723

Yes it is still reproducible: https://crash-stats.mozilla.com/report/index/bp-b563dcdc-a4c1-4761-b70a-683502120807

As noted in comment 0, it takes some trying to reproduce.

That said, if the resolution stated in comment 4 would resolve this bug, why isn't it done?  Certainly can't be a performance concern.
Comment 10 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-08-07 08:28:36 PDT
For anybody who wants to attempt this, first make sure you can reproduce the bug, then try adding null checks to http://hg.mozilla.org/mozilla-central/annotate/c8d94fe7506a/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#l1188 and http://hg.mozilla.org/mozilla-central/annotate/c8d94fe7506a/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#l1146 and see if you that fixes the crash.
Comment 11 Srinath N [:srinath] 2012-11-14 01:09:39 PST
Hi Josh, 

Per our discussion, I've put together a patch for this issue. Can you please assign the defect to me?
Comment 12 Srinath N [:srinath] 2012-11-14 18:10:19 PST
Created attachment 681804 [details] [diff] [review]
Patch for bug 683959 - Adding null checks for mDataStream

Patch for bug 683959 - Adding null checks for mDataStream
Comment 13 Srinath N [:srinath] 2012-11-14 18:15:38 PST
Hi IU,

I tested this using a Nightly on Win 8 and Win XP. I followed the steps as mentioned in the defect. I have not been able to reproduce the crash.

Per jdm's suggestion, I've made changes and have attached the patch to the defect. 

Can you kindly apply the patch and see if you are able to observe the crash?

Thanks in advance!
Comment 14 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-11-15 03:46:32 PST
You can get builds with the patch applied at https://tbpl.mozilla.org/?tree=Try&rev=a537cf62103f.
Comment 15 IU 2012-11-15 19:25:00 PST
(In reply to Josh Matthews [:jdm] from comment #14)
> You can get builds with the patch applied at
> https://tbpl.mozilla.org/?tree=Try&rev=a537cf62103f.

I can't run this build because Windows is complaining that I'm missing MSVCR100D.dll. I even installed Microsoft Visual C++ 2010 SP1 Redistributable Package and that did not help.

Is it possible to get a normal non-debug try build?
Comment 16 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-11-16 01:10:47 PST
It should be on its way at https://tbpl.mozilla.org/?tree=Try&rev=fbdba9064d66.
Comment 17 IU 2012-11-16 12:22:40 PST
Thanks.  I can confirm that the patch resolves the crashes.
Comment 18 Jason Duell [:jduell] (needinfo me) 2012-11-16 17:30:25 PST
Comment on attachment 681804 [details] [diff] [review]
Patch for bug 683959 - Adding null checks for mDataStream

Thanks for the patch! I've landed it:

  https://hg.mozilla.org/integration/mozilla-inbound/rev/76857b6b13e6
Comment 19 Ryan VanderMeulen [:RyanVM] 2012-11-17 05:13:43 PST
https://hg.mozilla.org/mozilla-central/rev/76857b6b13e6

Note You need to log in before you can comment on or make changes to this bug.