On Thu, Feb 09, 2023 at 06:58:14PM +0100, Omar Polo 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.

We should at most only wake up once because our events are not marked
with EV_PERSIST. The basic idea of this change is to skip the
gotd_imsg_event_add() call at the very bottom of gotd_request().
If that function is not called we should never be notified for
activity on the client's socket again.
During early development of gotd this has bitten me a bunch of times,
where everything appeared to stall because I essentially forgot to
call event_add() again.

> maybe we
> could just re-initialize the event in connect_session?  event_del +
> event_set (without event_add) should do it.

I don't know if doing this is safe while we are already being called
by libevent to handle another event. It may well be the case. But to
me it seems safer to "disable" an event by simply not adding it again
after it fires.