Improve php_openssl_sockop_set_option logic for liveness poll skipping

[bukka]

The idea is that we can skip poll for non blocking socket even of active ssl connection.

[MaxKellermann]

Additionally for blocking socket we don't use this optimization if SSL is active because in that case we're not using MSG_DONTWAIT

Are you sure this is a good idea? I've never used SSL_peek(), so I don't know if it's guaranteed to never block. The manpage doesn't mention explicitly; it only says:

SSL_peek_ex() and SSL_peek() are identical to SSL_read_ex() and SSL_read() respectively except no bytes are actually removed from the underlying BIO during the read

Since it's "identical", it should block just like SSL_read() does. The only difference, as written above, is that the returned data is not "consumed", but that's after having already blocked.

I'm not even sure if SSL_peek() is guaranteed to never block even if the socket is readable; what if there's just one byte in the socket, but not enough to finish the read operation? Is the existing code already broken (under certain, rare but remotely triggerable circumstances)?

[bukka]

SSL_peek will not block for non-blocking socket but instead it returns immediately and you can check the error. This is actually all documented in SSL_read docs:

If the underlying BIO is nonblocking, a read function will also return when the underlying BIO could not satisfy the needs of the function to continue the operation. In this case a call to SSL_get_error(3) with the return value of the read function will yield SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE. As at any time it's possible that non-application data needs to be sent, a read function can also cause write operations. The calling process then must repeat the call after taking appropriate action to satisfy the needs of the read function. The action depends on the underlying BIO. When using a nonblocking socket, nothing is to be done, but select() can be used to check for the required condition. When using a buffering BIO, like a BIO pair, data must be written into or retrieved out of the BIO before being able to continue.

We have got already logic for that and keep it alive if SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE error.

The part that I'm not sure about is how big optimization this really is. It will eliminate a poll if there is data but if there is not, there's probably some read inside SSL_peak and a bit of logic around it which might be slower. I might leave this open and do some experiments later with this though.

[bukka]

Also that poll is not exactly correct because in some situations we might need to wait for write instead. Although that might be a bit tricky to get right but probably something to look at in the future as well - this more a note for myself. :)