Closed
Bug 252118
Opened 20 years ago
Closed 20 years ago
necko ignores settings for max-persistent-connections-per-server
Categories
(Core :: Networking: HTTP, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.8beta1
People
(Reporter: sharkey, Assigned: darin.moz)
References
()
Details
(Keywords: fixed-aviary1.0, fixed1.7.5, regression, Whiteboard: [patch])
Attachments
(2 files, 2 obsolete files)
35.42 KB,
application/x-zip-compressed
|
Details | |
13.47 KB,
patch
|
bzbarsky
:
superreview+
asa
:
approval-aviary+
asa
:
approval1.7.5+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.7) Gecko/20040707 Firefox/0.9.2 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.7) Gecko/20040707 Firefox/0.9.2 A few days ago, I updated to Fx 0.9.2 after uninstalling 0.8. While everything was fine with 0.8, Fx 0.9 seems to ignore the individual settings for the # of max. concurrent downloads. No matter what number I put in my user.js for 'network.http.max-persistent-connections-per-server', Fx does not allow more than two concurrent downloads from the same server. When I type 'about:config', the settings are shown correctly (e.g. an integer value of 8 for the above mentioned), but Fx simply ignores them. Reproducible: Always Steps to Reproduce: 1. Install Fx 0.9.2 2. Go to www.mozilla.org/products/mozilla1.x/ for example. 3. Try to download all three different versions of Mozilla at the same time. Actual Results: The third download does not start at once, but only after the first one is finished or one of the first two is aborted. It is impossible to download more than two files from the same server concurrently. Expected Results: Start the third download immediately due to the settings in user.js & prefs.js: user_pref("network.http.max-persistent-connections-per-server", 8); The bug occurs independently of the theme in use and the installed extensions.
Important note: The problem is NOT caused by a server-side download restriction! A lot of people (including myself) have noticed that (see MozillaZine Forum for example), even on websites where Fx 0.8 or other browsers can download more than two files simultaneously.
Comment 2•20 years ago
|
||
This WFM with the Firefox 0.9.1 release build (win2000).
Keywords: regression
Assignee | ||
Comment 3•20 years ago
|
||
please provide a mozilla HTTP log per instructions here: http://www.mozilla.org/projects/netlib/http/http-debugging.html thanks!
Updated•20 years ago
|
Assignee: bugs → darin
Severity: major → normal
Component: Download Manager → Networking: HTTP
Product: Firefox → Browser
QA Contact: bmo → core.networking.http
Version: unspecified → Other Branch
Comment 4•20 years ago
|
||
http log set http://uk.altn.com/AntiVirus/Release/ to be home page started Firefox clicked /AntiVirus/Release/av224_en.exe, download started clicked /AntiVirus/Release/av224_fr.exe, download started clicked /AntiVirus/Release/av224_ge.exe, nothing happened canceled one of the downloads in progress, dialog for third download appeared and download commenced when OK'd
Comment 5•20 years ago
|
||
log was with a current aviary branch installer build on win2k. I've just gone back a bit - as I said, it worked for me using 0.9.1 release build, but using the aviary branch build from 20040703, it doesn't seem to work.
Just FYI... It seems that max-persistent-connections-per-server is locked at 2 and max-connections-per-server is locked at 8. You can toggle between the 2 behaviors by toggling keep-alive. I can provide supporting http traces as well as an Ethereal trace if it'll help further.
Here's some more information, as some users in the German firefox forum found out: By changing the values for max. connections and/or pipelining manually in the file all.js in the "Mozilla Firefox/greprefs" directory, Fx accepts those settings after the next restart and behaves like it should. Thus it seems that this is the only file which values are accepted. Changes made to the user.js, prefs.js or directly under about:config are shown correctly, but they do not have any effect.
Assignee | ||
Comment 8•20 years ago
|
||
Line 1552 of nsHttpHandler.cpp has this comment: // XXX should probably shutdown and init the conn mgr. So, it would seem that observing pref changes, which apply to connection management, is not implemented. If you hand edit the same pref in the all.js that ships with Firefox (inside the greprefs directory), then the pref would be honored. The solution to this bug is to shutdown and re-init the http connection manager. That should be a trivial patch.
Severity: normal → minor
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta
Assignee | ||
Comment 9•20 years ago
|
||
This should work. I haven't rigorously tested it yet, but it should do the trick. If you have the ability to compile Mozilla for yourself, then please give this patch a try. Thanks!
Assignee | ||
Updated•20 years ago
|
Attachment #154664 -
Flags: review?(cbiesinger)
Assignee | ||
Updated•20 years ago
|
Summary: Fx ignores settings for max-persistent-connections-per-server → necko ignores settings for max-persistent-connections-per-server
Assignee | ||
Comment 10•20 years ago
|
||
OK, I suspect this bug may have been triggered by subtle changes in the timing between profile selection and HTTP handler initialization. If it ever happened in the past that the profile was selected and prefs.js was parsed prior to the HTTP handler being initialized, then this bug would not be seen. (The exception is when modifying the pref via about:config ... it would not take affect until after a restart.) It seems that today in firefox, the HTTP handler is initialized before prefs.js has been parsed from the profile directory. It's unclear what is causing this. I'm not sure it is important to know why this was caused, since this bug exists when using about:config to change the pref during a browser session. Note: even if we have a lot of HTTP prefs in prefs.js, restarting the connection manager should be relatively inexpensive. Initialization is very cheap, amounting to little more than a call to do_GetService, and shutdown is likewise cheap when there are no connections or pending transactions to abort.
Assignee | ||
Updated•20 years ago
|
Whiteboard: [patch]
Updated•20 years ago
|
Attachment #154664 -
Flags: review?(cbiesinger) → review+
Comment 11•20 years ago
|
||
The ordering problem must have been fixed since FF 0.9.2 was released because the AVIARY nightlies will obey the settings if the brower is restarted. I just applied to the patch to my local AVIARY snapshot and it seems to have fixed the restart issue. thanks.
Assignee | ||
Updated•20 years ago
|
Attachment #154664 -
Flags: superreview?(bzbarsky)
Comment 12•20 years ago
|
||
Comment on attachment 154664 [details] [diff] [review] v1 patch So let me see if I understand this right. With this patch, if I change my accept-language pref that will cancel all http transactions that are in progress at the time?
Assignee | ||
Comment 13•20 years ago
|
||
> So let me see if I understand this right. With this patch, if I change my
> accept-language pref that will cancel all http transactions that are in
> progress at the time?
yes. do you think that is unacceptable?
a more involved version would call a new method on nsHttpConnectionMgr with the
updated parameter (or maybe all parameters in one batch). the conn mgr would
post an event to the socket thread containing the new parameter(s). that event
would then set the new parameters on the conn mgr. my fear is that the conn mgr
might get into a bad state if the parameters are reduced in value. at first
glance, i don't think that will be the case ;-)
Reporter | ||
Comment 14•20 years ago
|
||
Sorry, I am not a developer and thus my question may sound a bit dumb to you... but what was actually the reason to change the behaviour of the conn mgr from Fx 0.8 to 0.9? Wouldn't it be possible to simply implement the connection management as it was in 0.8?
Comment 15•20 years ago
|
||
Leaving it so that some of the connection prefs only take effect with a restart seems acceptable to me, if the solution is otherwise complicated. But that would bring us back to the problem you originally reported where the prefs don't get picked up on startup - the connection management hasn't actually changed to cause that bug. I guess that problem could be fixed elsewhere (in the profile loading stuff) instead? Unless it's got fixed already (I haven't tested, but that's what Remy said).
Comment 16•20 years ago
|
||
something like this patch is still needed, so that changes of the prefs take
effect immediately, not only after a restart.
> might get into a bad state if the parameters are reduced in value.
hm.. ideally it would not care, leave those connections open and don't accept
new ones while there are more connections than configured :) or it could kill
"idle" connections (keep-alive with no current request).
Why would this need to happen via an event on the socket transport thread?
Couldn't it just acquire a lock and set the member vars?
Assignee | ||
Comment 17•20 years ago
|
||
> something like this patch is still needed, so that changes of the prefs take > effect immediately, not only after a restart. agreed > > might get into a bad state if the parameters are reduced in value. > > hm.. ideally it would not care, leave those connections open and don't accept > new ones while there are more connections than configured :) or it could kill > "idle" connections (keep-alive with no current request). "ideally" sure.. but reality bites sometimes, and we have to make sure we do not introduce a regression ;-) > Why would this need to happen via an event on the socket transport thread? > Couldn't it just acquire a lock and set the member vars? what lock is that? multithreading is not so trivial... most of the conn mgr runs on the socket thread free of having to synchronize with locks (which is a huge benefit in that it greatly reduces the complexity of the conn mgr). the main thread communicates with the conn mgr via events posted to the socket thread for most everything... we have a monitor for the purpose of implementing synchronous shutdown, but that's about the only reason for a monitor in the conn mgr code.
Assignee | ||
Comment 18•20 years ago
|
||
OK, here's the "better" patch I spoke of. The only downside to this patch is that it adds a bit more code, but I think the fact that it won't cause connections to close when prefs change makes it the right choice ;-)
Attachment #154664 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #154664 -
Flags: superreview?(bzbarsky)
Assignee | ||
Updated•20 years ago
|
Attachment #154922 -
Flags: superreview?(bzbarsky)
Attachment #154922 -
Flags: review?(cbiesinger)
Comment 19•20 years ago
|
||
Comment on attachment 154922 [details] [diff] [review] v2 patch + nsresult UpdateParam(PRUint16 name, PRUint16 value); why not use a named enum for the name param? thanks for making this patch, I like this much more.
Attachment #154922 -
Flags: review?(cbiesinger) → review+
Assignee | ||
Comment 20•20 years ago
|
||
OK, with nsParamName...
Attachment #154922 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #154922 -
Flags: superreview?(bzbarsky)
Assignee | ||
Updated•20 years ago
|
Attachment #154957 -
Flags: superreview?(bzbarsky)
Comment 21•20 years ago
|
||
[on cancelling connections on pref changes]
> yes. do you think that is unacceptable?
Yes. Consider downloads....
Looking at the patch now.
Comment 22•20 years ago
|
||
Comment on attachment 154957 [details] [diff] [review] v2.1 patch sr=bzbarsky. I like this a lot more.
Attachment #154957 -
Flags: superreview?(bzbarsky) → superreview+
Assignee | ||
Comment 23•20 years ago
|
||
fixed-on-trunk for 1.8 alpha 3
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 24•20 years ago
|
||
There's yet another question in the Mozilla Firefox Support forum about this situation so I'd like to nominate this bug for blocking aviary1.0.
Flags: blocking-aviary1.0?
Comment 25•20 years ago
|
||
Remy: link? was that with a current branch build? last time I tried with a branch build, this worked for me.
Comment 26•20 years ago
|
||
(In reply to comment #25) > Remy: link? was that with a current branch build? last time I tried with a > branch build, this worked for me. Oops, sorry... http://forums.mozillazine.org/viewtopic.php?t=114760 The problem with settings not being saved correctly was fixed in the FF branch nightlies some time ago. Darin's fix for the settings being internally locked at their defaults currently isn't applied to the aviary branch though. I've been applying it to my own source tree for testing.
Assignee | ||
Updated•20 years ago
|
Attachment #154957 -
Flags: approval-aviary?
Comment 27•20 years ago
|
||
Comment on attachment 154957 [details] [diff] [review] v2.1 patch a=asa for branch checkin.
Attachment #154957 -
Flags: approval-aviary? → approval-aviary+
Comment 28•20 years ago
|
||
Comment on attachment 154957 [details] [diff] [review] v2.1 patch a=asa for 1.7 checkin too
Attachment #154957 -
Flags: approval1.7.x+
Updated•20 years ago
|
Flags: blocking-aviary1.0?
Comment 31•20 years ago
|
||
*** Bug 252903 has been marked as a duplicate of this bug. ***
Comment 32•20 years ago
|
||
*** Bug 264563 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•