Closed
Bug 898156
Opened 11 years ago
Closed 11 years ago
crash in Background thumbnail generation @ mozilla::net::FTPChannelParent::OnStartRequest
Categories
(Core Graveyard :: Networking: FTP, defect)
Tracking
(firefox24 unaffected, firefox25+ verified, firefox26 verified)
VERIFIED
FIXED
mozilla26
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)
21.36 KB,
application/zip
|
Details | |
37.25 KB,
text/x-log
|
Details | |
2.16 KB,
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.
Reporter | ||
Updated•11 years ago
|
Severity: normal → critical
Updated•11 years ago
|
Severity: critical → normal
Reporter | ||
Comment 1•11 years ago
|
||
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
Reporter | ||
Comment 2•11 years ago
|
||
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.
Reporter | ||
Comment 3•11 years ago
|
||
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 ?? ()
Reporter | ||
Comment 4•11 years ago
|
||
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
Reporter | ||
Comment 5•11 years ago
|
||
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.
Reporter | ||
Comment 6•11 years ago
|
||
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*) ]
Reporter | ||
Comment 7•11 years ago
|
||
(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.
Reporter | ||
Comment 8•11 years ago
|
||
Reporter | ||
Comment 9•11 years ago
|
||
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.
Updated•11 years ago
|
Component: General → Networking: FTP
Product: Firefox → Core
Comment 10•11 years ago
|
||
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).
Comment 11•11 years ago
|
||
(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.
Comment 12•11 years ago
|
||
FYI, we are seeing this signature on Windows as well, see https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Anet%3A%3AFTPChannelParent%3A%3AOnStartRequest%28nsIRequest*%2C%20nsISupports*%29
Updated•11 years ago
|
Whiteboard: [qa-automation-blocked] → [qa-automation-blocked][startupcrash]
Updated•11 years ago
|
Comment 13•11 years ago
|
||
This is the #11 crash on trunk and mostly within the first minute of running Firefox ("startup"), so marking this as topcrash.
Keywords: topcrash
Updated•11 years ago
|
Summary: Background thumbnail generation causes a crash on Ubuntu 12.04 32bit → crash in Background thumbnail generation @ mozilla::net::FTPChannelParent::OnStartRequest
Comment 14•11 years ago
|
||
jduell, can someone from your team investigate?
Flags: needinfo?(jduell.mcbugs)
Reporter | ||
Updated•11 years ago
|
Keywords: reproducible
Comment 15•11 years ago
|
||
is this related to or depends on bug 800347?
Comment 16•11 years ago
|
||
(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.
Comment 17•11 years ago
|
||
Do we have any URLs for this crash?
Reporter | ||
Comment 18•11 years ago
|
||
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?
Comment 19•11 years ago
|
||
see comment #2 for STR's
for URL's; nothing really useful but here's the top of the list. a bunch of 2 and 1 timers follow.
Total Count URL
13 about:sessionrestore
10 about:home
6 about:blank
6 about:addons
4 http://www.kandidatencheck.abgeordnetenwatch.de/
3 https://bsm.app.unimelb.edu.au/arsys/forms/bsm-api.app.unimelb.edu.au/SHR%3ALandingConsole/Default+Administrator+View/?cacheid=bd348dcc
3 http://www.google.nl/
3 https://get3.adobe.com/flashplayer/update/plugin/
3 about:newtab
3 https://plus.google.com/u/0/_/notifications/frame?sourceid=14&hl=ru&origin=http%3A%2F%2Fwww.google.com.ua&uc=1&jsh=m%3B%2F_%2Fscs%2Fabc-static%2F_%2Fjs%2Fk%3Dgapi.gapi.en.JjRB6e4QGsQ.O%2Fm%3D__features__%2Fam%3DEA%2Frt%3Dj%2Fd%3D1%2Frs%3DAItRSTP2_ANCFH40x
Comment 20•11 years ago
|
||
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.
Reporter | ||
Comment 21•11 years ago
|
||
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.
Reporter | ||
Comment 22•11 years ago
|
||
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.
Comment 23•11 years ago
|
||
FWIW I tried to reproduce using the steps in comment 2 and never could.
Reporter | ||
Comment 24•11 years ago
|
||
(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?
Comment 25•11 years ago
|
||
I'd be willing to give it a shot.
Reporter | ||
Comment 26•11 years ago
|
||
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)
Comment 27•11 years ago
|
||
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)
Comment 28•11 years ago
|
||
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?
Reporter | ||
Comment 29•11 years ago
|
||
(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.
Reporter | ||
Comment 30•11 years ago
|
||
(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.
Assignee | ||
Comment 31•11 years ago
|
||
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)
Assignee | ||
Comment 32•11 years ago
|
||
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.
Reporter | ||
Comment 33•11 years ago
|
||
Jason, would you also need the login credentials to test this live on that box? It's 100% reproducible there.
Assignee | ||
Comment 34•11 years ago
|
||
Sure, I'd love to be able to reproduce this on demand.
Reporter | ||
Comment 35•11 years ago
|
||
(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.
Assignee | ||
Comment 36•11 years ago
|
||
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)
Assignee | ||
Comment 37•11 years ago
|
||
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 38•11 years ago
|
||
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-
Comment 39•11 years ago
|
||
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.
Assignee | ||
Comment 40•11 years ago
|
||
Yeah, that seems like a good plan. I'll write a new patch. Thanks Patrick.
Flags: needinfo?(mcmanus)
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(mcmanus)
Assignee | ||
Comment 41•11 years ago
|
||
This also fixes the crash for me.
Attachment #801046 -
Attachment is obsolete: true
Attachment #802174 -
Flags: review?(mcmanus)
Comment 42•11 years ago
|
||
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+
Assignee | ||
Comment 43•11 years ago
|
||
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?
Assignee | ||
Comment 44•11 years ago
|
||
s/nonw/none/ :)
Assignee | ||
Comment 45•11 years ago
|
||
Filed bug 915024 for followup here to make FTP work correctly here.
Reporter | ||
Comment 46•11 years ago
|
||
(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?
Updated•11 years ago
|
Attachment #802174 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 47•11 years ago
|
||
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)
Comment 48•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Comment 49•11 years ago
|
||
status-firefox26:
--- → fixed
Reporter | ||
Comment 50•11 years ago
|
||
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.
Reporter | ||
Comment 51•11 years ago
|
||
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.
Flags: in-qa-testsuite?(hskupin)
Whiteboard: [qa-automation-blocked][startupcrash] → [mozmill][startupcrash]
Reporter | ||
Comment 52•11 years ago
|
||
(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)
Comment 53•11 years ago
|
||
The VM mm-ub-1204-32-4.qa.scl3.mozilla.com has been deleted from vmware and inventory(+rdns).
Flags: needinfo?(afernandez)
Reporter | ||
Comment 54•10 years ago
|
||
We take care of the test in bug 917205 so removing in-qa-testsuite.
Flags: in-qa-testsuite?(hskupin)
Updated•10 months ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•