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

[Andrew Bartlett]

On Fri, 2022-09-30 at 14:18 +0200, Stefan Metzmacher via samba-
technical wrote:
> Am 30.09.22 um 14:05 schrieb Stefan Metzmacher via samba-technical:
> > 
> > Is this plain tcp or via tls?

This is not TLS as far as I can see in the backtrace I was given
privately.  No TLS functions in the stack at least and port 389 as far
as I read it the netstat I was also given.

> 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
> 

Awesome debugging!

>                                  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.

How would we then, in this case, detect that the socket is closed?

While we would start processing other connections, wouldn't we change
to a not-busy wait never closing the socket as there is still data to
send but never getting to read the EOF?

How can we proceed to closing the socket?

> 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().

Do we have a good precedent for what the send timeout should be?

I can't see any good examples elsewhere in the code sadly.

One option is to reuse the search timeout, so that a query could at
most take 240sec, eg 2x 120sec, once for the search itself and once to
send over the network.  This would give only one knob to change (which
is good and bad). 

In the meantime I'm going to try and mock this up in a self-contained
cmocka test.

Andrew Bartlett
-- 
Andrew Bartlett (he/him)       https://samba.org/~abartlet/
Samba Team Member (since 2001) https://samba.org
Samba Team Lead, Catalyst IT   https://catalyst.net.nz/services/samba

Samba Development and Support, Catalyst IT - Expert Open Source
Solutions