Closed Bug 1497868 Opened 6 years ago Closed 5 years ago

Parent process hangs when playing a media from FTP server

Categories

(Core Graveyard :: Networking: FTP, defect, P2)

64 Branch
Desktop
Windows
defect

Tracking

(firefox64 wontfix, firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox64 --- wontfix
firefox65 --- fixed

People

(Reporter: emuser20140816, Assigned: dragana, NeedInfo)

Details

(Whiteboard: [necko-triaged])

Attachments

(3 files)

Steps to reproduce:
1. Put a media file (mp3, mp4 etc) on your FTP server
2. Access via Firefox browser
3. Spam reload (or back or close browser before the media is fully downloaded)

What happened?:
Parent process starts to eat CPU usage. When browser is exited, it will leave as zombie process.

Reproduced on:
Firefox Nightly 64.0a1 x64 (2018-10-09)
Firefox 62.0.3 x86
Component: General → Networking
Product: Firefox → Core
Hello Reporter,
Thanks for filing this bug.
Given it's reproducible on nightly and release, it's not a near regression.
I'm wondering that can you reproduce it in a HTTP server instead of FTP?
It helps to locate where the problem is.

Thanks!
Component: Networking → Networking: FTP
Flags: needinfo?(emuser20140816)
I've tried it on HTTP server in the same way but the problem wasn't reproducible.
This could only happen in FTP media playback. In particular when tab page is moved/reloaded while the media is partially loaded.
Attached video A video of procedure
A video of procedure is attached.
Flags: needinfo?(emuser20140816)
Can you please make a log: 
https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging
 please add nsFtp:5 to the list:
imestamp,rotate:200,nsHttp:5,cache2:5,nsSocketTransport:5,nsHostResolver:5 

thanks.
Flags: needinfo?(emuser20140816)
Assignee: nobody → dd.mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P2
Whiteboard: [necko-triaged]
Attached file FTP networking log
(In reply to Dragana Damjanovic [:dragana] from comment #4)
> Can you please make a log: 
> https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging
>  please add nsFtp:5 to the list:
> imestamp,rotate:200,nsHttp:5,cache2:5,nsSocketTransport:5,nsHostResolver:5 
> 
> thanks.

Log file is in attachment.
Flags: needinfo?(emuser20140816)
We used a buffered input for ftp. A pipe is created at:
https://searchfox.org/mozilla-central/source/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#1571

We never cancel it (https://searchfox.org/mozilla-central/source/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#2129), but that is ok because trying to read from the socket will return an error and the pipe will be closed. The problem appears when the pipe buffer is full: we check whether there is some place in the buffer(the pipe's buffer) and because there is no space we never access the socket and never pickup that the socket is closed.

I am not an expert in Pipes and AsyncCopiers.
Canceling pipe at nsFtpState::CloseWithStatus(nsresult status) works. Is it right approach. Would be possible that we have a similar problem elsewhere?
Flags: needinfo?(honzab.moz)
(In reply to Dragana Damjanovic [:dragana] from comment #6)
> We used a buffered input for ftp. A pipe is created at:
> https://searchfox.org/mozilla-central/source/netwerk/protocol/ftp/
> nsFtpConnectionThread.cpp#1571
> 
> We never cancel it
> (https://searchfox.org/mozilla-central/source/netwerk/protocol/ftp/
> nsFtpConnectionThread.cpp#2129), but that is ok because trying to read from
> the socket will return an error and the pipe will be closed. The problem
> appears when the pipe buffer is full: we check whether there is some place
> in the buffer(the pipe's buffer) and because there is no space we never
> access the socket and never pickup that the socket is closed.

OK, so at [1] we open the socket transport.  At [2] we open the input stream as buffered to read data from the socket and keep it in mDataStream, we keep input stream of the pipe and there is a stream copier between the socket and the pipe.  When the pipe is full, the stream copier will take care to wait for the pipe's output to free up and then poll the socket again for reading and restart feeding.  If stop reading from the pipe, it will hang, yes.

> 
> I am not an expert in Pipes and AsyncCopiers.
> Canceling pipe at nsFtpState::CloseWithStatus(nsresult status) works. Is it
> right approach. Would be possible that we have a similar problem elsewhere?

Closing the input is the right way (status or NS_BASE_STREAM_CLOSED will likely do the job).


[1] https://searchfox.org/mozilla-central/rev/876022232b15425bb9efde189caf747823b39567/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#1507-1512
[2] https://searchfox.org/mozilla-central/rev/876022232b15425bb9efde189caf747823b39567/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#1571-1576
Flags: needinfo?(honzab.moz)
Keywords: checkin-needed
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1de661bbcc0c
In nsFtp, close data pipe as well. r=mayhemer
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1de661bbcc0c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Flags: qe-verify+

I've tried to reproduce this issue, but it seems that it is quite intermittent on my machine; I was only able to see some firefox.exe processes left opened after closing the browser, but I cannot reproduce this constantly on Windows 7 x64. The CPU usage in my case have only spiked at about ~20%, even though I've tried to spam the refresh button for a couple of times. FileZilla was used, FWIW, for creating the FTP server.

Hi emuser20140816,

Since I was able to reproduce this bug only partially, and not regularly on my machine, could you please help us verify this bug and tell us if this build fix the issue on your end as well? Thank you!

Flags: needinfo?(emuser20140816)

Since there was no information received on the above question, after checking with Ryan VanderMeulen [:RyanVM], I am removing the qe verify+ flag here.

Flags: qe-verify+
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: