On 10/1/09 02:18, "Daniel Stenberg" <daniel_at_haxx.se> wrote:
> The connection cache should only keep actually open connections. As soon as
> libcurl detects a connection not being alive anymore it should be closed
> properly and removed from the connection cache. Anything else is a buggy
> behavior.
>
> But of course, the connections might've been alive when they were put into the
> cache and they all died while in there. Oh, but you said they were marked
> 'inuse' right? That can't be right as only connection still be actually in use
> should have that set. As soon as they are "returned" to the cache they should
> not be inuse anymore.

Right. I've noticed that this only happens when HTTPS over an NTLM
authenticating HTTP proxy is being used. Otherwise, sockets are reaped
immediately.

Additionally, I realized that we use the multi API without pipelining
enabled. When I turned it on, it all went to pot (our poll loop watching for
FDs that were closed, "Internal error 4" coming from cURL, etc. That's a
little surprising, because there shouldn't really be a reason for that
option so dramatically affecting cURL's behavior. But I wonder if it is part
of the problem.

FWIW, what I'm testing against is a Glib poll loop. I feel like that's
pretty typical.

> Hm. Well it could possibly be the case if your app somehow doesn't tell
> libcurl about the closing of the sockets and the terminating of the request,
> so that the socket is atually closed (as you see) but libcurl is still
> thinking the requests/connections are alive. This is just me speaking out my
> mind loud, I'm not exactly sure how it would occur.

Well, the poll loop should report POLLIN|POLLHUP status on a socket
disconnect. I'm pretty sure that's happening in the usual case, because I
instrumented the connection cache and it never reached full capacity when
either:

1) not using the NTLM proxy, but using HTTPS
2) using the NTLM proxy, but with the same server in HTTP mode

> No. NTLM is made for and only used for negotiating authentication for the
> _connection_. If the connection is closed there's no NTLM state left necesary
> to save as it needs to be restarted on the next connect.

Aha - that's what I suspected. You can ignore my comment about url.c then.
 
> The problem is rather that we can start a new connection and go with NTLM
> there but we can't keep it in the cache so we can't complete the NTLM
> negotiation. You can of course work around this by inreasing the connection
> cache size, but the actual problem is problem seen by this sympthom. The
> connection cache really shouldn't get full of inuse connections, and if that
> happens when it is about to do NTLM I think a better behavior would be to
> instead fail that transfer entirely and as early as possible as it simply
> won't be able to proceed with that.

That makes sense. Should I write a patch for that?

It seems like such a check could be vulnerable to race conditions, i.e.

create connection that needs NTLM
check connection cache for a free spot (possibly reaping some)
establish socket connection and send probe
>>> another transaction finishes up and lands a spot in the cache
initial NTLM packet sent, connection does not fit in cache, connection dies!

Or can you put a conn in the cache prior to even doing anything with it,
just to 'reserve' the spot?

-- 
Joshua Kwan
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html