As it is, EV_PERSIST is somewhat obtuse when trying to use it in combination
with EV_READ or EV_WRITE while still handling timeout events. The standard
idiom is basically:
if (select_epoll_etc_indicates_something)
do_something_and_add_fd_back_to_something_set();
else if (time_since_last_something > something_timeout)
our_client_or_server_is_dead_lets_cleanup();
One method is to do something like this with EV_PERSIST is:
void read_cb(int fd, short event, void *a)
{
opaque *obj = a;
ssize_t l;
switch (event) {
case EV_TIMEOUT:
close_cleanup_whatever(fd);
opaque_reset(obj);
event_del(obj->ev_r);
break;
case EV_READ:
l = recv(fd, ...);
/*
* Here we force a timeout reschedule. if we don't,
* our timeout will eventually trigger, regardless
* of the fact that we've been processing active events.
* It isn't really added with EV_PERSIST, just the timeout
* is rescheduled.
*/
event_add(obj->ev_r, obj->tout_recv_tv);
break;
default: break;
}
return;
}
Same goes for EV_SIGNAL and EV_TIMEOUT events. event_add(event, timeout),
always, if you want to reschedule the timer associated with it (which for
all intensive purposes you almost always will if you're using EV_PERSIST
in the first place).
Now it's not like this is a huge hassle, but it does seem like it would
be more inutitive for libevent to handle latching timers of this sort when
using EV_PERSIST. I've written a patch to handle latching timers when using
EV_PERSIST and currently I can see some pros and cons.
Pros:
1. EV_PERSIST + anything else works intuitively without having to worry
about managing the timeout one passed to event_add(). Set it and
forget it.
2. For some people it may be one less thing to truck around in their god
objects. I.e. the space taken up by storing the relative timer within
the event struct itself is balanced by the space saved by not carrying
it around in user objects. Not a gain, however, for people using
global timeout structs.
3. It's not really much of a boogeyman to current users of libevent as the
people who are using EV_PERSIST typically are aware of said timeout
issues. Also, the current workaround of event_add() will work fine
in existing code.
4. Focuses on optimizing the standard case rather than the exception (timeout).
Cons:
1. Cyclic timer still needs event_add() in the callback, as a timeout is
still setup to delete the event automatically. I'm torn on this. If
it's changed it will break current behaviour. If it isn't changed,
then it's another thing to remember when using the library - that is
events registered with EV_PERSIST will have their timeouts reset on
activity, however a timeout occuring will automatically delete the
event as per normal. In day to day usage this is probably a preferred
idiom though, even though it's an exception to the latching timer
pattern. One concern though is that since things are less explicit,
flow is not as self-evident.
2. Timeout rescheduling is done when event_active() is called. This is not
entirely precise because event timeouts may end up being rescheduled
to a slightly earlier moment in time. One way to get around this is to
move timeout_schedule() to event_process_active(), but it would take
some reworking of timeout_process().
3. Maybe this is all just a waste of time and we should just continue to use
event_add(obj, obj->tout);
With the model changed, it would look like this:
void read_cb(int fd, short event, void *a)
{
opaque *obj = a;
ssize_t l;
switch (event) {
case EV_TIMEOUT:
close_cleanup_whatever(fd);
opaque_reset(obj);
break;
case EV_READ:
l = recv(fd, ...);
/* etc */
break;
default: break;
}
return;
}
void dispatcher(opaque *obj)
{
event_set(obj->ev_r, obj->fd, EV_READ | EV_PERSIST, read_cb, obj);
event_add(obj->ev_r, obj->tout_recv_tv);
return;
}
Interested to hear thoughts before I go posting patches.
-cl
_______________________________________________
Libevent-users mailing list
[email protected]
http://monkeymail.org/mailman/listinfo/libevent-users
