Re: [Libevent-users] Re: Avoid potential SSL read spinlocks

[Catalin Patulea <catalinp@xxxxxxxxxx>]

On Mon, Nov 14, 2011 at 11:17 AM, Mark Ellzey <mthomas@xxxxxxxxxx> wrote:
> As far as I know, it doesn't. But I wrote this patch because if you are
> using a deferred bev with bulk incoming data, it never drops back into
> base_loop() until SSL_read() says there is nothing to read. This is due
> to the fact that your callback for new data is never called until
> libevent gets around to processing the deferred queue.
This is actually causing an assert in my case :( The key is to get
SSL_read to return both data (> 0) and EOF (= 0) within the same
iteration of that (former) while loop. Then the flow looks like this:

SSL_read() returns N bytes into an iov from evbuffer_reserve_space -
no run_read_cb yet
SSL_read() returns 0, which leads into conn_closed(), which queues a
error_cb(EOF)
Back in do_read, a read_cb is queued

In the event loop, the read_cb is delivered first, completing the HTTP
request. evhttp disables EV_READs and moves into state
EVCON_DISCONNECTED. ...and then the error_cb(EOF) is delivered,
confusing evhttp and causing assert(req != NULL) in
evhttp_connection_fail.

I've found it hard to point the finger at any particular piece of
code... It depends on whether the bufferevent interface defines an
ordering between EV_READ enable/disables and EOF/ERROR events.

In the case of bev_sock, once disable(EV_READ) has been called, the fd
is not checked for readability anymore, so read() doesn't have a
chance to report any EOF or errors. Effectively, we're leaving these
events queued in the kernel (which seems like a reasonable thing to do
- evhttp no longer cares about that fd).

Your patch indirectly does this - EOF/errors are now queued in OpenSSL
because SSL_read is only called once for every read_cb.

My only concern is that there may be cases where OpenSSL _needs_ us to
call SSL_read repeatedly to make any progress, despite the underlying
fd staying unreadable. Maybe during renegociation or application
payloads that span SSL records? The man page doesn't have anything to
that effect, but that doesn't mean the cases don't exist..

> This does not change any mechanics to openssl bufferevents without
> deferred enabled.
I agree that your patch doesn't affect non-deferred mechanics - but I
haven't been able to get evhttp to stack on top of bev_ssl without
deferred callbacks. It seems to send the request line, but gets stuck
in write_cb's before sending request headers. I haven't investigated
too much because it seems like it would only be a quick fix - sooner
or later someone may want to use deferred cbs with bev_ssl, for the
reasons described in the link you sent.

BTW, sample/le-proxy.c also enabled deferred cbs on its bev_ssl.
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.