On 11/21/2016 10:55 AM, bjt 3 wrote:
I've noticed an application creating a new connection for each request
in a set when the target is the same. In this set of requests, the
LOCALPORT/LOCALRANGE has been specified for each request but INTERFACE
has not. The application cares about the local port range, but really
doesn't care about the interfaces and is happy to use the default
value of NULL.
Some looking around leads me to:
lib/url.c ConnectionExists()
if(needle->localdev || needle->localport) {
/* If we are bound to a specific local end (IP+port), we must not
re-use a random other one, although if we didn't ask for a
particular one we can reuse one that was bound.
This comparison is a bit rough and too strict. Since the input
parameters can be specified in numerous ways and still end
up the
same it would take a lot of processing to make it really
accurate.
Instead, this matching will assume that re-uses of bound
connections
will most likely also re-use the exact same binding
parameters and
missing out a few edge cases shouldn't hurt anyone very much.
*/
if((check->localport != needle->localport) ||
(check->localportrange != needle->localportrange) ||
!check->localdev ||
!needle->localdev ||
strcmp(check->localdev, needle->localdev))
continue;
}
I would have guessed, prior to seeing this code, that a request for
any interface would have matched anything in the cache that matches
the other criteria. This code appears to explicitly exclude that
possibility and is the immediate cause of continual connection
reconstruction for the case that I'm seeing. Once seeing this, three
possibilities came to mind. needle->localdev being NULL should match
anything; needle->localdev and check->localdev both being NULL should
match; or, finally, that the code is correct and there are other
considerations at work here that weren't clear to me. Hence, this email.
My question is, is this coded as intended or overly restrictive ? If
the former, I'd very much like to understand why so as to improve my
understanding of libcurl.
interesting. this would loosen it a little:
diff --git a/lib/url.c b/lib/url.c
index 7106d46..5d0e21a 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -3400,9 +3400,9 @@ ConnectionExists(struct Curl_easy *data,
*/
if((check->localport != needle->localport) ||
(check->localportrange != needle->localportrange) ||
- !check->localdev ||
- !needle->localdev ||
- strcmp(check->localdev, needle->localdev))
+ ((check->localdev && needle->localdev) ?
+ strcmp(check->localdev, needle->localdev) :
+ (check->localdev || needle->localdev)))
continue;
}
-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
