On Fri, Sep 3, 2021 at 2:15 PM Ruediger Pluem <rpl...@apache.org> wrote: > > On 9/3/21 1:01 PM, Yann Ylavic wrote: > > > > What I want to avoid in this loop is to handle all the POLLIN|POLLERR > > or POLLOUT|POLLERR or POLLERR alone which depend on the system. > > So it's written such that there is no "blank" pass, when no > > POLLIN|POLLHUP or POLLOUT is returned (likely an error condition) we > > will issue either a read or a write (in that order of preference) to > > catch the error at the core filters' level. > > This was missing the POLLOUT|POLLHUP case when the socket is shutdown > > for read already, meaning that we will never get POLLIN anymore, and > > this is fixed by this !(reqevents & APR_POLLIN) case to issue a write > > (well a shutdown for write exactly here) when no read is to be issued > > below. > > > > Does it make sense? > > Just to be sure. We need this because > > 1. We requested POLLOUT > 2. We did not get POLLOUT back, but rtnevents might contain POLLHUP and or > POLLIN and hence the last condition existing before the > patch (!(pfd->rtnevents & (APR_POLLIN | APR_POLLHUP))) would not trigger. The > new condition ensures in these cases that we flush > stuff even if blocking and close down write.
We asked for POLLOUT (only) in reqevents but did not get it back in rtnevents, instead we got POLLHUP|POLLPRI (in the case of PR 65521) but it could also have been POLLERR or anything we didn't ask for. In any case when we ask for POLLIN or POLLOUT or both and get back none of them, the system tells us there is something "wrong" with the socket (error, reset or EOF). There might be something wrong with POLLIN or POLLOUT returned too but at least we know what to do when it happens, here we have no clue based on the returned events only. I think this can happen whether the connection was half shutdown or not, at least POLLERR, see below. > > This means we could also use > > tc->other->down_in == 1 > > instead of !(tc->pfd->reqevents & APR_POLLIN) > > to get the same results? This is a more restrictive check that would not catch for instance POLLERR while we asked for POLLOUT (only) on a full duplex connection still. We might have !(tc->pfd->reqevents & APR_POLLIN) because tc->other's output filters have pending data (we stop reading tc in this case to avoid yet more pending data and risk blocking), so it does not necessarily mean that the connections are half closed (down_in/out). Still if we get POLLERR we want to catch the error on write rather than read, because read is shutdown or suspended. > > Or does it need to be !(tc->other->pfd->reqevents & APR_POLLIN) instead of > !(tc->pfd->reqevents & APR_POLLIN) ? No, this is an event on tc (not tc->other), so we need to check whether tc needs reading and/or writing because we will read from and/or write to there in this pass. > > Still confused. Hope the above helps :/ Thanks; Yann.
