Closed Bug 898156 Opened 11 years ago Closed 11 years ago

crash in Background thumbnail generation @ mozilla::net::FTPChannelParent::OnStartRequest

Categories

(Core :: Networking: FTP, defect)

Product:
Core
Component:
Networking: FTP
Version:
25 Branch
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla26
Tracking Flags:
Tracking Status
firefox24 --- unaffected
firefox25 + verified
firefox26 --- verified

People

(Reporter: whimboo, Assigned: jduell.mcbugs)

References

Details

(5 keywords, Whiteboard: [mozmill][startupcrash])

Crash Data

Attachments

(3 files, 1 obsolete file)

places.sclite.zip
(deleted), application/zip
Details
http.log
(deleted), text/x-log
Details
v2: cancel during OnStartRequest if not FTP channel
(deleted), patch
mcmanus
: review+
akeybl
: approval-mozilla-aurora+
Details | Diff | Splinter Review 
As we have observed today, the new background thumbnail generation feature as landed with bug 870100 has been completely broken our Mozmill testruns on Mac and Linux. Instead of 213 tests only 2 tests are executed and Firefox shutdown right after. Flipping the pref "browser.pageThumbs.enabled" to false fixes the problem.

The first nightly build which showed the issue is 20130724030204. See the following results:

http://mozmill-daily.blargon7.com/#/functional/reports?branch=25.0&platform=Linux&from=2013-07-23&to=2013-07-25

What we see in the Jenkins console log is the following:

[mozilla-central_functional] $ mozmill-env/run mozmill-automation/testrun_functional.py --repository=mozmill-tests --junit=report.xml --report=$REPORT_URL builds/
creating 1!
[TabChild] SHOW (w,h)= (10, 10)
loading about:blank, 1
[Child 12816] ###!!! ABORT: ActorDestroy by IPC channel failure at LayerTransactionChild: file /builds/slave/m-cen-lx-ntly-0000000000000000/build/gfx/layers/ipc/LayerTransactionChild.cpp, line 83

###!!! [Child][AsyncChannel] Error: Channel error: cannot send/recv

*** Platform: Linux Ubuntu 12.04 32bit
*** Cloning repository to '/tmp/tmp0ScO_9.mozmill-tests'
requesting all changes
adding changesets
adding manifests
adding file changes
added 3297 changesets with 9575 changes to 775 files (+4 heads)
updating to branch default
407 files updated, 0 files merged, 0 files removed, 0 files unresolved
*** Installing 2013-07-24-22-27-11-mozilla-central-firefox-25.0a1.fr.linux-i686.tar.bz2 => /tmp/tmprsf5xQ.binary/
*** Application: Firefox 25.0a1
*** Updating to branch 'default'
pulling from mozmill-tests
searching for changes
no changes found
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
TEST-START | /tmp/tmp0ScO_9.mozmill-tests/tests/functional/testLayout/testNavigateFTP.js | setupModule
TEST-PASS | /tmp/tmp0ScO_9.mozmill-tests/tests/functional/testLayout/testNavigateFTP.js | testNavigateFTP.js::setupModule
TEST-START | /tmp/tmp0ScO_9.mozmill-tests/tests/functional/testLayout/testNavigateFTP.js | testNavigateFTP
TEST-PASS | /tmp/tmp0ScO_9.mozmill-tests/tests/functional/testLayout/testNavigateFTP.js | testNavigateFTP.js::testNavigateFTP
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed
INFO Passed: 2
INFO Failed: 1
INFO Skipped: 0

This dubious output I have tracked down to the following range:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=599fe516bed5&tochange=96d374c8f833

So only bug 870100 comes into my mind here.
Severity: normal → critical
Severity: critical → normal
Version: 23 Branch → 25 Branch
I have to correct myself a bit. While the above message appears on both Mac and Linux we only completely fail on Ubuntu 12.04 32bit. 

The exact console output is:
creating 1!

[TabChild] SHOW (w,h)= (10, 10)
loading about:blank, 1
TEST-PASS | /tmp/tmpKo9tkj.mozmill-tests/tests/functional/testLayout/testNavigateFTP.js | testNavigateFTP.js::testNavigateFTP
[Child 8693] ###!!! ABORT: ActorDestroy by IPC channel failure at LayerTransactionChild: file /builds/slave/m-cen-lx-ntly-0000000000000000/build/gfx/layers/ipc/LayerTransactionChild.cpp, line 83

###!!! [Child][AsyncChannel] Error: Channel error: cannot send/recv
Actually this is a crash of Firefox which I can reproduce manually by doing the following steps:

1. Install the latest tinderbox debug build
2. Start Firefox with a fresh profile and open ftp://ftp.mozilla.org/pub
3. Click on 'Firefox'
4. Click on 'Nightly'

At latest with step 4 Firefox crashes.
Severity: normal → critical
Keywords: assertion, crash
Summary: Background thumbnail generation breaks Mozmill testruns on Mac and Linux → Background thumbnail generation causes a crash on Ubuntu 12.04 32bit
Lots of warnings are reported when I let the tinderbox build run under gdb:

[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(mMainThread) failed: file ../../../xpcom/threads/nsThreadManager.cpp, line 252
[Child 9516] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0xC1F30001: file nsThreadUtils.cpp, line 161
[Child 9516] WARNING: NS_ENSURE_TRUE(compMgr) failed: file nsComponentManagerUtils.cpp, line 58
[Child 9516] WARNING: NS_ENSURE_TRUE(svc) failed: file ../../../dom/ipc/ContentChild.cpp, line 565
[Child 9516] WARNING: NS_ENSURE_TRUE(compMgr) failed: file nsComponentManagerUtils.cpp, line 58
creating 1!
[Child 9516] WARNING: nsWindow::GetNativeData not implemented for this type: file ../../../widget/xpwidgets/PuppetWidget.cpp, line 673
++DOCSHELL 0x83dbad8 == 1 [id = 1]
++DOMWINDOW == 1 (0x84566e0) [serial = 1] [outer = (nil)]
[TabChild] SHOW (w,h)= (10, 10)
loading about:blank, 1
++DOMWINDOW == 2 (0x84b96e8) [serial = 2] [outer = 0x84566e0]
[Parent 9481] WARNING: Could not get disk information from DiskSpaceWatcher: file ../../../../dom/src/storage/DOMStorageIPC.cpp, line 324

Program received signal SIGSEGV, Segmentation fault.
0xfc9de940 in ?? ()
Finally I hit the crash in a Nightly build, but I'm not sure if those are related to each other. At least Firefox crashes all the time now after some seconds without doing anything.

Crash report: bp-d7e10530-a587-4d99-b0e1-de3a62130725

1 	libxul.so 	mozilla::net::FTPChannelParent::OnStartRequest(nsIRequest*, nsISupports*) 	netwerk/protocol/ftp/FTPChannelParent.cpp
2 	libxul.so 	mozilla::net::nsHttpChannel::CallOnStartRequest() 	netwerk/protocol/http/nsHttpChannel.cpp
3 	libxul.so 	mozilla::net::nsHttpChannel::ContinueProcessNormal(tag_nsresult) 	netwerk/protocol/http/nsHttpChannel.cpp
4 	libxul.so 	mozilla::net::nsHttpChannel::ProcessNormal() 	netwerk/protocol/http/nsHttpChannel.cpp
5 	libxul.so 	mozilla::net::nsHttpChannel::ProcessResponse() 	netwerk/protocol/http/nsHttpChannel.cpp
Blocks: 898194
I have to note that when accessing the FTP server we are currently forced to use the squid proxy in SCL3, which outputs the content as HTML. This might be related here.
The behavior here is strange. Here some updated information:

Whenever this crash happens and I keep the profile, Firefox will always crash a couple seconds after the restart with the same crash over and over again. That happens as long as I don't move away the thumbnails folder in the profile. But when I do that, the crash stops. Also it will come back once I revert the move of the folder and it will be present again in the profile. I tried to check that profile at home outside of the proxy environment, but I'm not able to reproduce. So it looks like that the squid proxy is really necessary here.

I have taken a look at crash-stats and it shows this crash also on other platforms:

https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Anet%3A%3AFTPChannelParent%3A%3AOnStartRequest%28nsIRequest*%2C+nsISupports*%29

I will check some of those URLs if I can reproduce it without the proxy environment.
Crash Signature: [@ mozilla::net::FTPChannelParent::OnStartRequest(nsIRequest*, nsISupports*) ]
(In reply to Henrik Skupin (:whimboo) [away 07/29 - 08/11] from comment #6)
> again. That happens as long as I don't move away the thumbnails folder in
> the profile. But when I do that, the crash stops. Also it will come back
> once I revert the move of the folder and it will be present again in the

That was actually not true. Instead it is related to the places.sqlite file in that profile. When I remove it the crash stops happening. I will attach a copy of that file in a bit.
Attached file places.sclite.zip (deleted) — 

    
    

    
  
The reason why it crashes immediately after restart are the entries for ftp.mozilla.org, which are located in the table moz_places. Given that we create new thumbnails of most visited pages in the background now, Firefox will find that URL because it's the only one I have opened for that profile.

With all that we really fail in the creation of thumbnails in the background when a proxy is involved. And given that the generation happens in the background, I assume that the actual crash signature and stack is not the one for the thread we want to have.

    
  
Component: General → Networking: FTP
Product: Firefox → Core

    
    

    
  
I've several errors with the current nightly, like:
[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMessageSender.sendAsyncMessage]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/BackgroundPageThumbs.jsm :: <TOP_LEVEL> :: line 276"  data: no]

and the plugin container is continuously crashing (I don't know if it's related).

    
    

    
  
(In reply to Marco Castelluccio [:marco] from comment #10)
> the plugin container is continuously crashing (I don't know if it's related).

Only way to know for sure I guess would be to back out the changes in bug 870100 from your local build.

    
  
Whiteboard: [qa-automation-blocked] → [qa-automation-blocked][startupcrash]

    
    

    
  
This is the #11 crash on trunk and mostly within the first minute of running Firefox ("startup"), so marking this as topcrash.
Keywords: topcrash

    
  
Summary: Background thumbnail generation causes a crash on Ubuntu 12.04 32bit → crash in Background thumbnail generation @ mozilla::net::FTPChannelParent::OnStartRequest

    
    

    
  
jduell, can someone from your team investigate?
Flags: needinfo?(jduell.mcbugs)
Keywords: reproducible

    
    

    
  
is this related to or depends on bug 800347?

    
    

    
  
(In reply to Tracy Walker [:tracy] from comment #15)
> is this related to or depends on bug 800347?

Doubtful.

Jason, I'm terrified by nsHttpChannel::CallOnStartRequest somehow calling into FTPChannelParent::OnStartRequest.

    
    

    
  
Do we have any URLs for this crash?

    
    

    
  
Just to mention it again, we were able to reproduce this constantly in our mozmill-ci cluster with the affected Ubuntu machines. The URLs in the crashreport are most likely not those you are interested in, because we fail when retrieving thumbnails in a background thread. And those we do not collect, right?

    
    

    
  
Bug 881641 has the same basic signature.  I'm guessing the content thrown up by squid (rather than it being directly related to FTP) along with OMTC is the issue.

    
    

    
  

      
      
      
      

        Attached file
          
        http.log (deleted)
        — 
      

    


  
Ok, so I tried to reproduce on one of our CI machines and it failed immediately with the latest nightly build. So I was going ahead and created an HTTP log with timing information. I hope that helps to investigate the problem.

What's interesting is that we are trying to open the FTP connection via port -1. Not sure if that is used for auto-detect, or if that is the problem.

    
    

    
  
It's very sad to see that no further action happens on this bug. We will reach beta soon, so there is a much higher chance that way more people will be involved with this crash. Jason, if you can't work on it, who else could step in? Thanks.

    
    

    
  
FWIW I tried to reproduce using the steps in comment 2 and never could.

    
    

    
  
(In reply to Josh Matthews [:jdm] from comment #23)
> FWIW I tried to reproduce using the steps in comment 2 and never could.

Adrian, most likely can easily create a new VM from our Ubuntu 12.04 32bit template in qa.SCL3.mozilla.com. So you would have a machine to work on. I could prepare everything which would be necessary to get started. How does that sound?

    
    

    
  
I'd be willing to give it a shot.

    
    

    
  
That's great. Adrian, can you please deploy a new Ubuntu 12.04 32bit from the template? We need this to investigate and fix a top crasher in Firefox. Thanks!
Flags: needinfo?(afernandez)

    
    

    
  
Was not sure on hostname or if this would be a temporary host but created the following VM: mm-ub-1204-32-4.qa.scl3.mozilla.com

If you need the hostname changed, please let us know.

On a side note, if you are planning to make some base changes, let us know and we could clone/template those changes in case you need to reproduce the issue.
Flags: needinfo?(afernandez)

    
    

    
  
Looking at the stack in comment 12, it looks like the line at http://hg.mozilla.org/mozilla-central/annotate/a4c1961bf723/netwerk/protocol/http/nsHttpChannel.cpp#l1003, which is:

nsresult rv = mListener->OnStartRequest(this, mListenerContext);

Ends up calling into mozilla::net::FTPChannelParent::OnStartRequest(nsIRequest*, nsISupports*)

nsHttpChannel is passing |this|, a |nsHttpChannel *|, but the FTPChannelParent does a static_cast<nsFtpChannel*> on this.  IOW, it appears to be trying to cast an nsHttpChannel to an nsFtpChannel, which is clearly not correct.

Looking at the http.log attachment in this bug seems to verify something like that is happening:
--
2013-08-27 08:37:16.532587 UTC - -1220507904[b721a240]: Creating HttpBaseChannel @a02a9000
2013-08-27 08:37:16.532591 UTC - -1220507904[b721a240]: Creating nsHttpChannel [this=a02a9000]
2013-08-27 08:37:16.532607 UTC - -1220507904[b721a240]: HttpBaseChannel::Init [this=a02a9000]
2013-08-27 08:37:16.532611 UTC - -1220507904[b721a240]: host=ftp.mozilla.org port=-1
2013-08-27 08:37:16.532614 UTC - -1220507904[b721a240]: uri=ftp://ftp.mozilla.org/
2013-08-27 08:37:16.532624 UTC - -1220507904[b721a240]: nsHttpChannel::Init [this=a02a9000]
...
--

Note how opening ftp://ftp.mozilla.org/ is creating an nsHttpChannel.  Digging a little deeper, the one place I could imagine this happening is at http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/ftp/FTPChannelParent.cpp#88

This code takes a URI and creates a channel which it assumes (but never checks) is an ftp channel.  It then calls:

rv = mChannel->AsyncOpen(this, nullptr);

If mChannel there was actually an nsHttpChannel instead of an nsFtpChannel, we would then end up with the callstack in comment 12.  And the fact the log implies we are indeed ending up with an nsHttpChannel for a ftp:// URI makes me think this might be worth exploring.

So I guess the question I'm asking is - can anyone explain how an attempt to open an ftp:// URL could actually open a HTTP channel?  Is there anything the squid proxy could do to the response to make that happen?

    
    

    
  
(In reply to Mark Hammond (:markh) from comment #28)
> So I guess the question I'm asking is - can anyone explain how an attempt to
> open an ftp:// URL could actually open a HTTP channel?  Is there anything
> the squid proxy could do to the response to make that happen?

That's totally correct Mark! As mentioned earlier on this bug we are seeing this with a Squid proxy in-between. When we are trying to connect to an FTP server as of now, the proxy 'http://proxy.dmz.scl3.mozilla.com:3128' is used, so that an HTTP connection gets created. The content which comes back has also been transformed to HTML (but that might be unrelated).

We most likely hit this by accident because we want to get rid of Squid and have a direct ftp connection. See the work happening on bug 880709 (if you can't open it let me know and I will CC you). Meanwhile I think we should keep at least one of our tests, so we could have a testcase for this crash - not sure if it can be an unit test. Please let me know about.

(In reply to Adrian Fernandez [:Aj] from comment #27)
> Was not sure on hostname or if this would be a temporary host but created
> the following VM: mm-ub-1204-32-4.qa.scl3.mozilla.com
> 
> If you need the hostname changed, please let us know.

I think that's fine for now. We most likely don't have that VM that long. I will prepare everything and will comment later with the login credentials.

    
    

    
  
(In reply to Henrik Skupin (:whimboo) from comment #29)
> > Was not sure on hostname or if this would be a temporary host but created
> > the following VM: mm-ub-1204-32-4.qa.scl3.mozilla.com
> > 
> > If you need the hostname changed, please let us know.
> 
> I think that's fine for now. We most likely don't have that VM that long. I
> will prepare everything and will comment later with the login credentials.

Josh, in case you need the IP address of this host, it is: 10.22.73.22. The credentials I have sent via email.

Everything is prepared via the ´crash´ profile. As of now you can already see the crash with a debug tinderbox build used in '~/firefox/firefox'. When you restart Firefox just wait a couple of seconds and Firefox will crash immediately.

    
    

    
  
I've had trouble reproducing this too--(I got it to happen once, but couldn't capture it again in a debugger).  But given comment 28 I strongly suspect the issue is that when an HTTP proxy is in use for FTP, we redirect the FTP request to an HTTP one:

  http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#2242

We need to make that work correctly in the medium/long run, but for right now I'm guessing that we'll be fine just detecting in the child that an HTTP proxy is in use and failing the FTP channel right at AsyncOpen time (I assume the thumbnail service will simply move on to some other URI to thumbnail if asyncOpen fails.  And ftp-over-http-proxy is not a common use case for B2G). I'll write a patch that does this tomorrow, and file a bug for a proper fix.
Assignee: nobody → jduell.mcbugs
Flags: needinfo?(jduell.mcbugs)

    
    

    
  
It also looks like we could replace the static_cast with a QI--the only place we need static_casts are from IPDL types, not an nsIRequest.  I'll look into that--I'm not sure how we'll calculate lastModifiedTime from an nsIHttpChannel, but we must be doing something for that case in the non-IPDL proxyied FTP case.  I'll look into it.

    
    

    
  
Jason, would you also need the login credentials to test this live on that box? It's 100% reproducible there.

    
    

    
  
Sure, I'd love to be able to reproduce this on demand.

    
    

    
  
(In reply to Jason Duell (:jduell) from comment #34)
> Sure, I'd love to be able to reproduce this on demand.

Email with credentials has been sent.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        898156.ftp_e10s_crashfix.v1 (obsolete) (deleted)
        — 
          — Splinter Review
      

    


  
Turns out a full working implementation of e10s handling FTP->HTTP redirects will take more work than I expect we want to jam into aurora.

This patch just detects whether we're going to use an HTTP proxy for FTP, and returns an error from AsyncOpen if so (only for e10s FTP).  Patrick, the only reason I'm having you review this is that you know the proxy settings--am I OK just checking network.proxy.ftp and network.proxy.ftp_port, or do I also need to check network.proxy.type?  I'm guessing we only set the ftp/ftp_port vars when we're using an HTTP proxy (vs SOCKS), so that's enough of a test.

        Attachment #801046 -
        Flags: review?(mcmanus)

    
    

    
  
Comment on attachment 801046 [details] [diff] [review]
898156.ftp_e10s_crashfix.v1

With this patch a profile that crashes 100% of the time now doesn't crash.

I'd do a try run but of course we have no FTP tests :)

    
    

    
  
Comment on attachment 801046 [details] [diff] [review]
898156.ftp_e10s_crashfix.v1

Review of attachment 801046 [details] [diff] [review]:
-----------------------------------------------------------------

unfortunately checking the config won't work.. there are all kinds of other ways a proxy could  be assigned. PAC is the big black box.. but "system config" is pretty much the same kind of buck passing. and then there is the "use proxy for all protocols", but then there are exception lists for all of those. its too feature rich.

I'll chime in if I can think of a way that will work.

        Attachment #801046 -
        Flags: review?(mcmanus) → review-

    
    

    
  
could you just qi the nsIRequest in ftpchannelparent::onstartrequest() to be ftp.. and if it isn't ftp just cancel the request in the parent, fake something to send back to the child in onstartrequest, and then wait for the cancel to bubble a real error back to the child through onstoprequest?

hand waving required.

    
    

    
  
Yeah, that seems like a good plan.  I'll write a new patch.  Thanks Patrick.
Flags: needinfo?(mcmanus)

    
  
Flags: needinfo?(mcmanus)

    
    

    
  


  
This also fixes the crash for me.

        Attachment #801046 -
        Attachment is obsolete: true

        Attachment #802174 -
        Flags: review?(mcmanus)

    
    

    
  
Comment on attachment 802174 [details] [diff] [review]
v2: cancel during OnStartRequest if not FTP channel

Review of attachment 802174 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/ftp/FTPChannelParent.cpp
@@ +190,5 @@
>  {
>    LOG(("FTPChannelParent::OnStartRequest [this=%p]\n", this));
>  
> +  nsCOMPtr<nsIChannel> chan = do_QueryInterface(aRequest);
> +  NS_ENSURE_TRUE(chan, NS_ERROR_UNEXPECTED);

I don't know that we should really be throwing an error on that case.. but its fine for now because its clearly better than the old code that just statically cast it

        Attachment #802174 -
        Flags: review?(mcmanus) → review+

    
    

    
  
Comment on attachment 802174 [details] [diff] [review]
v2: cancel during OnStartRequest if not FTP channel

https://hg.mozilla.org/integration/mozilla-inbound/rev/9fbdf3af5feb

[Approval Request Comment]
Bug caused by (feature/regressing bug #): longstanding bug in e10s FTP when using proxy, surfaced by out of process thumbnailing.
User impact if declined: users who use an HTTP proxy will crash if they visit FTP URIs (when the thumbnail service tries to use FTP in child process with proxy) 
Testing completed (on m-c, etc.):  fixes crash for me (we have no automated FTP test infrastructure).  I suggest we watch crash rates for a few days.
Risk to taking this patch (and alternatives if risky): low: pretty clean workaround (just fail channel in this case)
String or IDL/UUID changes made by this patch: nonw

        Attachment #802174 -
        Flags: approval-mozilla-aurora?

    
    

    
  
s/nonw/none/ :)

    
    

    
  
Filed bug 915024 for followup here to make FTP work correctly here.

    
    

    
  
(In reply to Jason Duell (:jduell) from comment #43)
> Testing completed (on m-c, etc.):  fixes crash for me (we have no automated
> FTP test infrastructure).  I suggest we watch crash rates for a few days.

We can get a mozmill test added for this specifically. That shouldn't be a problem given that all of our infrastructure is behind the squid proxy. Would you like to have such a test?

Jason, do you still need the Ubuntu box or can we get it deleted by IT?

    
  

        Attachment #802174 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

    
    

    
  
Henrik, can you verify that we're not seeing the mozmill crashes any more with this patch landed?  I'd like to verify that before we land on aurora.

re: FTP testing: it sounds like mozmill must already be hitting ftp.mozilla.org? (otherwise we wouldn't have a crash). In that case I guess we now do have some sort of test coverage for FTP (at least for purposes of this crash).  If there's a way to get FTP testing that actually verified the content that's received, that would be great too (but right now we don't have an in-tree FTP server we could run on localhost--is it ok to hit ftp.mozilla.org in a testsuite? I thought that was discouraged).  Thanks for offering to look into this--FTP could really use testing.

I don't need the account on the ubuntu box any more--managed to duplicate this on my local box.
Flags: needinfo?(hskupin)

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/9fbdf3af5feb
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26

    
    

    
  
It works great with Nightly. So verified fixed with Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130912030201 CSet: a4e9c9c9dbf9

The latest Aurora version is from before the landing, so I have to verify the fix tomorrow.
Status: RESOLVED → VERIFIED

          status-firefox26:
          fixed → verified
Flags: needinfo?(hskupin)

    
    

    
  
Also verified fixed on Aurora with Mozilla/5.0 (X11; Linux i686; rv:25.0) Gecko/20100101 Firefox/25.0 ID:20130916004002 CSet: f70b04dc58f4

Looks like we are good to go with re-enabling our existing Mozmill tests. I will file a new bug for a specific crash test of this scenario. I can't promise when we will have it.

          status-firefox25:
          fixed → verified
Flags: in-qa-testsuite?(hskupin)
Whiteboard: [qa-automation-blocked][startupcrash] → [mozmill][startupcrash]

    
    

    
  
(In reply to Adrian Fernandez [:Aj] from comment #27)
> Was not sure on hostname or if this would be a temporary host but created
> the following VM: mm-ub-1204-32-4.qa.scl3.mozilla.com

Adrian, can you please remove this VM? We don't need it anymore after this top crasher has been fixed. Thanks a lot.
Flags: needinfo?(afernandez)
Blocks: 917205

    
    

    
  
The VM mm-ub-1204-32-4.qa.scl3.mozilla.com has been deleted from vmware and inventory(+rdns).
Flags: needinfo?(afernandez)

    
    

    
  
We take care of the test in bug 917205 so removing in-qa-testsuite.
Flags: in-qa-testsuite?(hskupin)

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: