Thanks for your report.

AFAICS from the backtrace alone, the thread is waiting until data come from a socket.  Is there any specific reason to believe there is a bug in m2crypto?

(As #507903 demonstrates, almost anything could be caused by a bug in M2Crypto - but this backtrace is roughly what I'd expect from an ordinary server that has currently nothing to do.)

Having a backtrace with most local variables would be ideal; knowing the parameters of the interrupted select() and read() system calls, as well as possible information about the referenced files (e.g. is it a local file, pipe, or a socket?) would probably be helpful as well.

while waiting for data (that never came), the thread blocked other threads forever. This reminded me of bug 472690 of m2crypto.

However, unlike back then, we saw the issue only once. I don't have answers to your questions but maybe the core I've set you reveals a few.

The other threads are blocked in select(); if they were blocked on a lock, the backtrace should be showing futex().

Can you try to obtain a complete backtrace from the core, please?  I have tried on a RHEL5 machine, and it didn't show anything useful.

Created attachment 428196 [details]
fullest bt that I could get

This bug is rather rare, but it happened again. It seems that Thread 1 is stuck in a never-ending ssl_accept(). Until accept returns, my process cannot accept any new connection, obviously.

I'd appreciate if you find what blocks there for hours, and make it fail within a reasonable timeout.

It seems the created sockets are blocking, in which case they correctly do what they should - block.  You can set a timeout for all operations on a socket using SSL.Connection.settimeout(); be prepared to handle the resulting SSLTimeoutError exceptions.

Tomas, could you look at the backtrace, please?  I originally suspected the client just isn't sending anything - actually the server is already in SSL3_ST_SR_FINISHED_A, and blocking (per danken for hours) waiting for a new TLS record.  Apart from figuring out the cause, if you could suggest how to write a client that reproduces such behavior, that would be helpful as well for testing the timeout handling.

The client would have to send the ChangeCipherSpec message but then not send the Finished message and just wait instead. Unfortunately I think that writing such client would require calls to openssl internal functions.

I did not look at the problem in depth but it seems to me this is just an application coding error. I don't quite understand why blocked SSL_accept() in one thread should block other threads from processing their requests - especially the main server listening loop. The thread running the SSL_accept should be separate from the main server listening loop - the thread should be created immediately after the accept() syscall.

(In reply to comment #5)
> It seems the created sockets are blocking, in which case they correctly do what
> they should - block.  You can set a timeout for all operations on a socket
> using SSL.Connection.settimeout(); be prepared to handle the resulting
> SSLTimeoutError exceptions.

I am already using SSL.Connection.settimeout() on the listening socket. Could it be that the timeout is not "inherited" by by the client socket returned by socket.socket.accept()? 

(In reply to comment #6)
> 
> I did not look at the problem in depth but it seems to me this is just an
> application coding error. I don't quite understand why blocked SSL_accept() in
> one thread should block other threads from processing their requests -
> especially the main server listening loop. The thread running the SSL_accept
> should be separate from the main server listening loop - the thread should be
> created immediately after the accept() syscall.    

Please note that my code is almost identical to M2Crypto.SSL.Connection.accept(), where SSL_accept() is called by the same thread that calles accpet().

Then I'd say, this is a not-quite-correct implementation in M2Crypto either.

SSL_accept() might definitely block depending on the client's behavior.

(In reply to comment #7)
> (In reply to comment #5)
> > It seems the created sockets are blocking, in which case they correctly do what
> > they should - block.  You can set a timeout for all operations on a socket
> > using SSL.Connection.settimeout(); be prepared to handle the resulting
> > SSLTimeoutError exceptions.
> 
> I am already using SSL.Connection.settimeout() on the listening socket. Could
> it be that the timeout is not "inherited" by by the client socket returned by
> socket.socket.accept()? 
Argh, you are right.  I was so focused on finding a specific reproducer that I missed this possibility.

On second thought, changing this in m2crypto is not a good idea.

Looking at http://bugs.python.org/issue7995 , the Python socket module (for which SSL.Connection() is supposed to be a plug-in replacement where possible) is sets all sockets returned from socket.accept() as blocking.  (Indeed, if the timeout were inherited by TCP sockets managed by the socket module, the timeout would have already been inherited by the current m2crypto code.)

Second, making a socket non-blocking inside SSL.Connection.accept() wouldn't be useful.  If the non-blocking operation times out, the user should later repeat the operation - but SSL.Connection.accept() would call socket.accept() again, getting a _new_ client socket instead of continuing negotiation with the existing client.

So, a non-blocking server needs to use a manually written replacement for SSL.Connection.accept(), and vdsm already does this.  That is also the right place to make the client socket non-blocking.
        sock, addr = self.socket.accept()
        ssl = SSL.Connection(self.ctx, sock)
add
        ssl.settimeout(...)

I'll have to follow your suggestion and ssl.settimeout() before ssl.accept_ssl() as I reimplement SSL.Connection.accept() already.

However, I think that m2crypto should do that too. socket.socket.accept() on a non-blocking socket never blocks. SSL.Connection() which is supposed to be a plug-in replacement ;-) should have the same semantic. It should simply make sure that it always returns a non-blocking.
To me it seems like the reasonable behavior: accept() timed out, and the user does not really care if it happened before or after tcp handshake. In the latter case, the intermediate client socket goes to waste; too bad, just cut your losses and close it.

That's not how TCP sockets work.

With TCP sockets, the handshake occurs in the background, and accepts() only waits until some handshake finishes and creates a socket for the connection.  A non-blocking accept(), or accept() interrupted with a signal, does not interrupt any handshakes in the background.

A program can therefore use poll()/select() with non-blocking accept() calls to reliably handle all incoming requests (within the limits of the queue length set by listen()).

Similarly, in the Python API non-blocking sockets and/or timeouts are "restartable" - if a read() or accept() times out, you can always try again later.


openssl does not work that way: the socket must be first created using regular accept(), an openssl data structure must be created for it, and only then is SSL_accept() used to perform the SSL negotiation.  This negotiation occurs in the foreground, with the calling thread blocking for I/O.  The calling process must therefore separately integrate accept() and SSL_accept() into their poll()/select() loop; there is no way to make the change transparent.

For true non-blocking sockets (settimeout(0)) such a program change is absolutely necessary: try adding settimeout(0) to your modified implementation - no connection will ever be accepted.

It would be possible to support non-zero timeouts within Connection.accept, dropping the connection - but that would be completely inconsistent with the other cases.