]> granicus.if.org Git - libevent/commitdiff
Avoid potential SSL read spinlocks
authorMark Ellzey <mark.thomas@mandiant.com>
Mon, 14 Nov 2011 15:24:07 +0000 (10:24 -0500)
committerNick Mathewson <nickm@torproject.org>
Mon, 14 Nov 2011 16:52:51 +0000 (11:52 -0500)
OpenSSL bufferevents with deferred callbacks enabled under high load will
spinlock in the function consider_reading(). This loop continues until all
data has been read.

Because of this condition; openssl bufferevents will never return back into
event_base_loop() until SSL_read has determined data is no longer ready.

As of yet I have not found a reason why this while loop exists, so this patch
just swaps out while for if.

If needed I can write same code which would trigger this effect; optionally
libevhtp has a test.c program which can be run with the following flags:

./test -s <keyfile.pem>

curl -vvvv -k -d@<HUGE_ASS_FILE> https://127.0.0.1:8081/

The return data will include the number of times the readcb got data and the
length of that read.

Without this patch, you are likely to see a small amount of "bytes read....",
otherwise the "bytes read..." return data should show much more reasonable
numbers.

bufferevent_openssl.c

index 86a8619b0c2196d5c5743026ed0ccc846b549056..3843b314e4f713c2a8555e4049cf5bb1a1577a2b 100644 (file)
@@ -722,15 +722,13 @@ consider_reading(struct bufferevent_openssl *bev_ssl)
        }
        if (bev_ssl->write_blocked_on_read)
                return;
-       while ((bev_ssl->bev.bev.enabled & EV_READ) &&
+       if ((bev_ssl->bev.bev.enabled & EV_READ) &&
            (! bev_ssl->bev.read_suspended) &&
            (! wm->high || evbuffer_get_length(input) < wm->high)) {
                int n_to_read =
                    wm->high ? wm->high - evbuffer_get_length(input)
                             : READ_DEFAULT;
                r = do_read(bev_ssl, n_to_read);
-               if (r <= 0)
-                       break;
        }
 
        if (!bev_ssl->underlying) {