1329089 - File upload blocks the following XHR requests in LAN

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: log.txt-main.5594.zip

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161208153507

Steps to reproduce:

0. prepare a webserver for uploading and responsing status at "LAN"
1. use XHR post to upload a big file (2GB+)
2. use XHR to get the status of uploading from server periodically  <== NG, no response until file had been uploaded

Please find the about:networking log file at attachment.




Actual results:

The second, third, forth, ... requests are blocked until file had been uploaded.


Expected results:

The second, third, forth, ... requests should be sent to server asap....

Comment 1

[John Hu [:johnhu][:johu][:醬糊小弟]]

We found this issue at a testing LAN environment with server hardware. Those two devices are connected with gigabits network. The reproduce ratio is 100% in this network.

If we move the same settings to slower network environment, the reproduce ratio is pretty low. But, it is still reproducible in a slower LAN, like 100MB, but not 100%.

Comment 2

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: screen shot of web console when the issue happened

This attachment is the screen shot of web console when the issue happened.

The "stream" request is the file uploading XHR. Once this one had issued, the other requests, like "status", "history", "session" are pended until 26 seconds later.

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

BTW,

We can reproduce this issue at Firefox 50.1.0 and Nightly..

Comment 4

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

From the nspr log in attachment, STS thread is trapped in busy loop writing HTTP request to socket output stream.
https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpConnection.cpp#1692

The first nsSocketOutputStream::AsyncWait is hit after uploading 500MB of the file.

Comment 5

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

We can try breaking this loop by limiting the total bytes we can write in one OnSocketWritable call. In this case STS thread will be able to process more transactions in parallel.

Comment 6

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff0722d3d2d0

Comment 7

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: Bug 1329089 - yield to other transactions while reaching the max write attempts.

Review commit: https://reviewboard.mozilla.org/r/102884/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102884/

Comment 8

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8824362 [details]
Bug 1329089 - yield to other transactions while reaching the max write attempts.

limit the maximum loop count per OnSocketWritable, so that STS thread will not be blocked by one large HTTP request.

Comment 9

[Patrick McManus [:mcmanus]]

Comment on attachment 8824362 [details]
Bug 1329089 - yield to other transactions while reaching the max write attempts.

thanks. I actually think simpler is better here given the corner case; drop the pref and just hardcode a number. It will be much less code. Even as low as 128 might be worth trying.

Comment 10

[John Hu [:johnhu][:johu][:醬糊小弟]]

SC,

I had downloaded the test version from your server and do the 10+ tests. This issue disappeared in this test build.

Please try to push to Firefox beta if possible.

Thank you very much.....

Comment 11

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8824362 [details]
Bug 1329089 - yield to other transactions while reaching the max write attempts.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102884/diff/1-2/

Comment 12

[Patrick McManus [:mcmanus]]

Comment on attachment 8824362 [details]
Bug 1329089 - yield to other transactions while reaching the max write attempts.

https://reviewboard.mozilla.org/r/102884/#review103958

lgtm thanks.. I don't think this meets uplift criteria fwiw

Comment 13

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

(In reply to Patrick McManus [:mcmanus] from comment #12)
> Comment on attachment 8824362 [details]
> Bug 1329089 - yield to other transactions while reaching the max write
> attempts.
> 
> https://reviewboard.mozilla.org/r/102884/#review103958
> 
> lgtm thanks.. I don't think this meets uplift criteria fwiw

uplift to beta may be too exaggerated, how about aurora?

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49414a3861de
yield to other transactions while reaching the max write attempts. r=mcmanus

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/49414a3861de

Comment 16

[Patrick McManus [:mcmanus]]

this is a tail use case - it can ride the trains and doesn't meet the uplift criteria imo

Comment 17

[Sylvestre Ledru [:Sylvestre]]

cf comment #16