Just for interest ran my h2 load tests on trunk/2.4.x unpatched and with
commenting ERR_clear_error(); in ssl_engine_io.c
On my Mac I see no differences, really. trunk used openssl 1.0.2o and 2.4.x had
1.0.2l:
> h2load -i gen/load-urls-1.txt -n 200000 -t 4 -m 100 -c <clients>
trunk
raw, 128 clients: 33940.36 req/s
patched, 128 clients: 33699.89 req/s
2.4.x
raw, 128 clients: 33417.12 req/s
patched, 128 clients: 34071.83 req/s
(just some runs, nothing with any statistical relevance)
Caveat: this does not necessarily say anything about the http/1.1 performance.
h2 buffers its writes into he ssl output filters more than h1 does, I think.
Cheers, Stefan
> Am 29.08.2018 um 09:55 schrieb Ruediger Pluem <[email protected]>:
>
>
>
> On 08/29/2018 09:37 AM, Christophe Jaillet wrote:
>> Pure speculation:
>>
>> Actually we ERR_clear_error unconditionally so that in case of error, we can
>> safely call SSL_get_error.
>>
>> Doc says:
>> <<<<<<<<<< >>>>>>>>>>
>> ERR_get_error() returns the earliest error code from the thread's error
>> queue and removes the entry. This function can be called repeatedly until
>> there are no more error codes to return.
>>
>> ERR_peek_error() returns the earliest error code from the thread's error
>> queue without modifying it.
>>
>> ERR_peek_last_error() returns the latest error code from the thread's error
>> queue without modifying it.
>> <<<<<<<<<< >>>>>>>>>>
>>
>> Couldn't we avoid the ERR_clear_error call (which looks expensive according
>> to the thread), and:
>> - loop on SSL_get_error to empty the error queue, in case an error
>> occurred and we want la latest one?
>> or
>> - do a ERR_peek_last_error() / ERR_clear_error() in the error handling
>> path (if we really care about clearing the error queue)
>>
>> IMHO, these 2 solutions would do the same as the current code, without
>> requiring ERR_clear_error in the normal case.
>
> Unfortunately it isn't that easy, because SSL_get_error does not only look at
> the error stack but also at the return
> code from a read / write call passed to it. It quickly breaks out with two
> possible error return values if something
> is on the error stack and this what caused the addition of the
> ERR_clear_error call. We had the situation that another
> consumer of openssl in the code base left an error unhandled on the stack and
> thus caused SSL_get_error to draw the
> wrong conclusions. While we could argue the error is in the other part of the
> code and the other consumer of openssl
> should clear up the error stack correctly I don't think this is the complete
> solution, because
>
> 1. We should not fail if a another consumer is bogus. We should be prepared
> for others to have bugs regarding this.
> 2. As the error handling is a stack I think it is a valid use case for
> another consumer of openssl to check for the
> error later and not directly like e.g. with errno.
>
> I am not sure yet if it is not SSL_get_error that makes wrong assumptions
> about the error stack, but that would be
> something hard to solve for us.
>
> Regards
>
> Rüdiger
