On 2023/02/09 18:58:14 +0100, Omar Polo <op@omarpolo.com> wrote:
> On 2023/02/09 17:19:25 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > Not a functional change. But while trying to figure out why gotd
> > was crashing in libevent, this stood out as a potential suspect
> > of libevent misuse.
> 
> it took me a bit to see that this is safe.  It will still delete the
> event in disconnect().
> 
> it's certainly better than calling event_del in multiple places, but
> this also means that we could be awakened to dispatch on gotd_request
> multiple times for events that we're not interested in.  maybe we
> could just re-initialize the event in connect_session?  event_del +
> event_set (without event_add) should do it.
> 
> it's also not clear to me if we could still have something queued for
> sending in connect_session.  msgbuf_clear will delete pending data.
> could still be useful to end up in gotd_request to flush that data.
> 
> so, in short, i think this improves the current situation but not sure
> if it does so in the right direction ^^".

actually, the idea there is to forward the fd to the other process, so
we _need_ to "clean" it somehow.  pending data shouldn't be there, but
the event is still registered and could still trigger (not sure how
kqueue/poll and dup'ed sockets interact.)  So, i think that the
current event_del call is the right place.  can't we use the fd == -1
check in disconnect to avoid the event_del there?