Closed Bug 948757 Opened 6 years ago Closed 6 years ago
Preconnect on hover: unlimited connections to the same host
Go to any non-ssl page, such as http://planet.mozilla.org/ Hover a link, Firefox opens a TCP preconnection. Now hover the same link again (move the mouse back and forth quickly): more and more connections get open to the same host. This can easily exceed network.http.max-persistent-connections-per-server If there are many links together, it's easy to do an unintentional SYN flood.
thanks - I can confirm on nightly. I'm guessing that it is seer related...
As mentioned via email - we were blindly ignoring any idle connections, instead of counting them towards the parallel speculative connection limit like we should have been. This patch is a little more liberal than the strictest version (which would sum the number of half opens & the number of idles) in that it checks both separately, meaning we could conceivably end up with 2x the speculative connection limit (either 12 or 2 connections, depending on if it's from the seer or not, respectively). The strict way is an easy modification to this patch. I also made the seer not override any limits for hovering predictions for extra safety, and in retrospect, it doesn't make nearly as much sense to override limits for hovers as it does for active pageload predictions.
Assignee: nobody → hurley
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 8346227 [details] [diff] [review] 0001-Bug-948757-Don-t-flood-servers-with-bunches-of-speculative-connections.-r-mcmanus.patch Review of attachment 8346227 [details] [diff] [review]: ----------------------------------------------------------------- I agree that hover should stay at 1 - so that nullptr change is good. we can remove all remnants of ignoreIdle from the IDL and such too, right? I'll leave the r? unresolved for that. I think we ought to pref off seer in 28 as well and just get the whole set of related things fixed up on the trunk rather than backporting all this.
(In reply to Patrick McManus [:mcmanus] from comment #3) > we can remove all remnants of ignoreIdle from the IDL and such too, right? > I'll leave the r? unresolved for that. *facepalm* of course. New patch incoming. > I think we ought to pref off seer in 28 as well and just get the whole set > of related things fixed up on the trunk rather than backporting all this. Agreed. I'll update bug 948569 to be for both beta & aurora.
Now with even more code removal!
Attachment #8346611 - Flags: review?(mcmanus) → review+
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/5e499ab5e49d for causing browser-chrome bustage like this: https://tbpl.mozilla.org/php/getParsedLog.php?id=31890027&tree=Mozilla-Inbound on at least Windows 7 and 8.
Here's the version that will be checked in (which doesn't time-out mochitest-bc, see https://tbpl.mozilla.org/?tree=Try&rev=89345c52aa28). This fixes the specific predict on link hover issue, but doesn't fix the underlying issue (which also needs fixed). I'll file a follow-up for that work.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Could you please verify that this is fixed for you in Firefox 29?
You need to log in before you can comment on or make changes to this bug.