On 7/10/20 11:54 AM, Yann Ylavic wrote:
> On Fri, Jul 10, 2020 at 9:55 AM Ruediger Pluem <[email protected]> wrote:
>>
>> On 7/1/20 6:35 PM, [email protected] wrote:
>>> Author: ylavic
>>> Date: Wed Jul 1 16:35:48 2020
>>> New Revision: 1879401
>>>
>>> for (i = 0; i < nresults; i++) {
>>> - const apr_pollfd_t *cur = &results[i];
>>> - int revents = cur->rtnevents;
>>> + const apr_pollfd_t *pfd = &results[i];
>>> + struct proxy_tunnel_conn *tc = pfd->client_data;
>>> +
>>> + ap_log_rerror(APLOG_MARK, APLOG_TRACE8, 0, r,
>>> + "proxy: %s: #%i: %s: %hx/%hx", scheme, i,
>>> + tc->name, pfd->rtnevents, tc->pfd->reqevents);
>>>
>>> /* sanity check */
>>> - if (cur->desc.s != client->pfd->desc.s
>>> - && cur->desc.s != origin->pfd->desc.s) {
>>> + if (pfd->desc.s != client->pfd->desc.s
>>> + && pfd->desc.s != origin->pfd->desc.s) {
>>> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10222)
>>> "proxy: %s: unknown socket in pollset",
>>> scheme);
>>> rc = HTTP_INTERNAL_SERVER_ERROR;
>>> goto cleanup;
>>> }
>>> -
>>> - in = cur->client_data;
>>> - if (revents & APR_POLLOUT) {
>>> - in = in->other;
>>> - }
>>> - else if (!(revents & (APR_POLLIN | APR_POLLHUP))) {
>>> + if (!(pfd->rtnevents & (APR_POLLIN | APR_POLLHUP |
>>> APR_POLLOUT))) {
>>> /* this catches POLLERR/POLLNVAL etc.. */
>>> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10220)
>>> "proxy: %s: polling events error (%x)",
>>> - scheme, revents);
>>> + scheme, pfd->rtnevents);
>>> rc = HTTP_INTERNAL_SERVER_ERROR;
>>> goto cleanup;
>>> }
>>> - out = in->other;
>>>
>>> - ap_log_rerror(APLOG_MARK, APLOG_TRACE8, 0, r,
>>> - "proxy: %s: #%i: %s/%hx => %s/%hx: %x",
>>> - scheme, i, in->name, in->pfd->reqevents,
>>> - out->name, out->pfd->reqevents, revents);
>>> + if (pfd->rtnevents & APR_POLLOUT) {
>>> + struct proxy_tunnel_conn *out = tc, *in = tc->other;
>>> +
>>> + ap_log_rerror(APLOG_MARK, APLOG_TRACE8, 0, r,
>>> + "proxy: %s: %s output ready",
>>> + scheme, out->name);
>>> +
>>> + rc = ap_filter_output_pending(out->c);
>>> + if (rc == OK) {
>>> + /* Keep polling out (only) */
>>> + continue;
>>> + }
>>> + if (rc != DECLINED) {
>>> + /* Real failure, bail out */
>>> + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
>>> APLOGNO(10221)
>>> + "proxy: %s: %s flushing failed (%i)",
>>> + scheme, out->name, rc);
>>> + goto cleanup;
>>> + }
>>> + rc = OK;
>>> +
>>> + /* No more pending data. If the input side is not readable
>>> + * anymore it's time to shutdown for write (this direction
>>> + * is over). Otherwise back to normal business.
>>> + */
>>> + del_pollset(pollset, out->pfd, APR_POLLOUT);
>>> + if (in->readable) {
>>> + ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r,
>>> + "proxy: %s: %s resume writable",
>>> + scheme, out->name);
>>> + add_pollset(pollset, in->pfd, APR_POLLIN);
>>> + out->writable = 1;
>>> + }
>>> + else {
>>> + ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, r,
>>> + "proxy: %s: %s write shutdown",
>>> + scheme, out->name);
>>> + apr_socket_shutdown(out->pfd->desc.s, 1);
>>> + }
>>> + }
>>>
>>> - if (in->readable && (in->drain || !(revents & APR_POLLOUT))) {
>>> + if (pfd->rtnevents & (APR_POLLIN | APR_POLLHUP)
>>
>> Why do we check for APR_POLLHUP here as well? I noticed a case where we
>> request only APR_POLLOUT but get back APR_POLLOUT and
>> APR_POLLHUB which causes an endless loop.
>
> Hmm, POLLHUP and POLLOUT are mutually exclusive. When requesting for
> POLLOUT only, I don't think we can get POLLHUP alone either, because
> even if the peer shut down (FIN) the connection we should still be
> able to write, and a RST would cause POLLERR (presumably since a
> read() would return an error, as opposed to POLLHUP's read() would
> return EOF).
>
> Is the loop you are thinking about when we get POLLHUP (alone, so
> don't enter "flushing" in POLLOUT code) and then
> ap_proxy_transfer_between_connections() does nothing (because of
> pending output data)?
ap_proxy_transfer_between_connections returned EOF in this case.
> As explained above, I don't think it's a possible state, but maybe we
> should handle it as POLLERR just in case..
I observed 36 which 0x24 or POLLHUP | POLLOUT for rtnevents and
52 which is 0x34 or POLLHUP | POLLOUT | POLLERR for rtnevents
Regards
Rüdiger
