On Tue, Nov 13, 2007 at 06:27:58PM -0800, William Ahern wrote: > > event_set(ev, fd, EV_READ | EV_PERSIST, read_cb, obj); > > event_add(ev, &timeout); > > > > read_cb() will be called whenever a read event happens, and it's timeout > > as passed to event_add() will be reset to the original value you passed. > > You do not have to call event_add() within the handler. > > > > event_set(ev, -1, EV_TIMEOUT | EV_PERSIST, timer_cb, obj); > > event_add(ev, &cycle); > > > > timer_cb() will be called when timeout (as passed via cycle) expires. It > > will then reschedule itself with it's original timeout, e.g. periodic timer. > > You do not have to call event_add() within the handler. > > What was the original behavior? Did the timer disappear? If not, I fail to > see how the new behavior is more justificed than the old; meaning, instead > of changing it, why not add it as an option? If waiting to read an entire > line, but you are trickled one byte at a time, do you really want to reset > the timeout between bytes, or is the timeout meant to limit the time spent > on reading the line?
The original behaviour with EV_PERSIST was this: 1. Implicitly not delete our event, but do not reschedule the timer. 2. Merrily read/write/signal/etc. on an event and then out of the blue receive EV_TIMEOUT because the timeout was never rescheduled unless you explicitly did it (which one probably won't inutitively expect with EV_PERSIST). We went over this one before though. The issue is that the "attacker crafting 1 byte data to keep connection open for eternity" thing, while valid - I don't really see how it's a libevent issue. That's an application issue. Otherwise, with EV_PERSIST, the timeout added to the event no longer plays the role of a timeout once a single event happens on said event, and the timeout is not rescheduled manually. The concept of a timeout being tied to "if we get a single unit of y bytes" means the timeout is then a sideband module within said event now. However, receive 1 byte == event occured - and based on - that timeout should no longer be relevant to the situation that already occured. The example you bring up of wanting to read y units of bytes in a given time is still possible without EV_PERSIST, or by using an associated timer event (probably not preferred), etc. It's just that the previous example is so much an exception to the standard idiom of: wait_until_event_or_timeout(); do_stuff() || delete_myself(); reset_timeout(); With EV_PERSIST as it is, it's only convenient for exceptional cases rather than the norm - and that's the focus of this patch - to reverse that. -cl > > > For the event_del() changes, it's just moving event_del() into > > event_active(), > > when an event occurs. There is a feature request on sourceforge for this, > > and this couples nicely with the EV_PERSIST change. It also allows us to > > rexamine the logic tests within the various event dispatchers themselves, as > > event_active() will only delete the same event once. > > Will this cause incompatabilities? What if you call event_add() anyhow? Will > it return failure? This could silently break code. Maybe the user set > EV_PERSIST, realized it didn't do what he wanted, but never removed it. event_add() will then reschedule the event based on the timeout you passed to it. But since it's already implicit with EV_PERSIST, it would just be a redudant but safe thing to do. -cl _______________________________________________ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
