Subject: Re: libssh2_session_free changes nonblock state of already unused fd

Re: libssh2_session_free changes nonblock state of already unused fd

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Fri, 28 Oct 2011 11:42:25 +0200 (CEST)

On Fri, 28 Oct 2011, Pavel Strashkin wrote:

> You're correct, but as i said we have the deal with a Perl, Python, whatever
> else that has GC which doesn't guarantee anything to you. An object can be
> destroyed at an time.

It guarantees a lot and please don't come blaming the language for this bug.

The socket is not yours to close as long as you've handed it over to libssh2
so doing so is a bug. perl certainly doesn't force you to. Do you need to make
some extra little quirk or precaution to make sure this happens correctly?
Perhaps. But this way of working is very common in transfer libraries written
in C so this is certainly not the first time and it is not the first perl
binding that needs to solve this problem. Just cleanup the libssh2 handle
first, then close the socket.

> In this world you should you take care of it too (in case if it happens). My
> point here (no hacks, no workarounds) is that you shouldn't change IO from
> blocking to async.

Why not? You hand over the socket to the library to do SSH operations on it.
Why isn't libssh2 then allowed to do things with the socket in ways that
allows it to perform as good as possible? What bad does it cause your program
if it changes some socket options?

> You have the stream (socket) and the only thing which you should have to do
> is just send and recv.

That's an easy thing to say from an outsider's perspective before you have
given the problem and the existing API some proper thought. I'd urge you to
read up on the code and the API again and then think again and if you then
still believe this to be true, then by all means write a patch and try it out
and see for yourself.

> How it happens - async or blocking - is not up to library. I mean you have
> to support both cases as you do already,

No, we don't. We always and unconditionally use the socket set to non-blocking
internally and we offer an optionally blocking API on top of that non-blocking
socket.

> This small change will make life easier i believe.

I disagree. This non-blocking issue is what made you notice your bug. You
still closed the socket before you told libssh2 to let it go and that was the
problem. libssh2 is entitled to do whatever it deems correct with the socket
until you've told it to give the socket back. Changing libssh2's treatment of
blocking vs non-blocking does not fix your bug.

-- 
  / daniel.haxx.se
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2011-10-28