Hi Christopher, I created a tracker item for this at:
https://sourceforge.net/tracker/?func=detail&atid=461325&aid=1831173&group_id=50884 Let's discuss potential resolutions there. Thank you, Niels. On Nov 13, 2007 1:24 AM, Christopher Layne <[EMAIL PROTECTED]> wrote: > 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 > Libevent-users@monkey.org > http://monkeymail.org/mailman/listinfo/libevent-users > > _______________________________________________ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
