How to detect a client-closed connection during a write from our LDAP server?

Stefan Metzmacher metze at samba.org
Fri Sep 30 12:18:32 UTC 2022


Am 30.09.22 um 14:05 schrieb Stefan Metzmacher via samba-technical:
> Am 30.09.22 um 14:00 schrieb Stefan Metzmacher via samba-technical:
>> Am 30.09.22 um 13:31 schrieb ronnie sahlberg via samba-technical:
>>> On Fri, 30 Sept 2022 at 21:12, Isaac Boukris via samba-technical
>>> <samba-technical at lists.samba.org> wrote:
>>>>
>>>> On Fri, 2022-09-30 at 14:26 +1300, Andrew Bartlett via samba-technical
>>>> wrote:
>>>>> I've been trying to chase down the CPU spins reported by our users in
>>>>> the writev() codepath from our LDAP server.
>>>>>
>>>>> A private mail the the strace output shows the sockets are in
>>>>> CLOSE_WAIT state, returning EAGAIN over and over (after a call to
>>>>> epoll() each time).  That alone would be enough to keep things
>>>>> spinning.
>>>>>
>>>>> But they are being shut down, however our LDAP server won't be
>>>>> triggering a read any time soon, it is waiting to flush the response
>>>>> out.
>>>>
>>>> Why wouldn't it? I mean why does the read waits for the write? if epoll
>>>> says so then we should read, then we may detect that the client closed
>>>> it end and may decide to give up the writes.
>>>>
>>>> Technically, I think there is no problem to write to a socket after the
>>>> peer sent us FIN,
>>>
>>> I think possibly the problem is that IF the tcp tx buffer is full, you
>>> might get -EAGAIN instead of a better error
>>> if the socket is only half-closed.
>>
>> The question is why does epoll report EPOLLOUT in that case.
>> If we don't get that and get EPOLLERR|EPOLLHUP we should
>> disable reporting unless TEVENT_FD_READ is requested.
>>
>> See epoll_event_loop() and epoll_handle_hup_or_err().
> 
> Is this plain tcp or via tls?

Ah, we register tstream_bsd_fde_handler() with TEVENT_FD_READ|TEVENT_FD_WRITE
and have this logic:

static void tstream_bsd_fde_handler(struct tevent_context *ev,
                                     struct tevent_fd *fde,
                                     uint16_t flags,
                                     void *private_data)
{
         struct tstream_bsd *bsds = talloc_get_type_abort(private_data,
                                    struct tstream_bsd);

         if (flags & TEVENT_FD_WRITE) {
                 bsds->writeable_handler(bsds->writeable_private);
                 return;
         }
         if (flags & TEVENT_FD_READ) {
                 if (!bsds->readable_handler) {
                         if (bsds->writeable_handler) {

=============> here we have the loop

                                 bsds->writeable_handler(bsds->writeable_private);
                                 return;
                         }
                         TEVENT_FD_NOT_READABLE(bsds->fde);
                         return;
                 }
                 bsds->readable_handler(bsds->readable_private);
                 return;
         }
}

We call the writeable handler (which is non-blocking) when we get TEVENT_FD_READ
because we assume it will get an error if TEVENT_FD_READ was generated by
EPOLLERR. I think moving TEVENT_FD_NOT_READABLE(bsds->fde); before
if (bsds->writeable_handler) would make sure we only try that path once
and not many times in a busy loop.

And in ldapsrv_call_writev_start() we may want to use tevent_req_set_endtime() on
the subreq of tstream_writev_queue_send(), so that ldapsrv_call_writev_done()
will get ETIMEDOUT from tstream_writev_queue_recv() and call ldapsrv_terminate_connection().

metze




More information about the samba-technical mailing list