Re: [Libevent-users] Event priorities

[Ralph Castain <rhc@xxxxxxxxxxxx>]

On Apr 30, 2012, at 4:38 PM, Nick Mathewson wrote:

> On Mon, Apr 30, 2012 at 4:48 PM, Ralph Castain <rhc@xxxxxxxxxxxx> wrote:
>> 
>> On Apr 30, 2012, at 2:30 PM, Nick Mathewson wrote:
>> 
>>> On Mon, Apr 30, 2012 at 3:53 PM, Ralph Castain <rhc@xxxxxxxxxxxx> wrote:
>>>> Hi folks
>>>> 
>>>> I'm working with priorities in events using libevent 2.0.13. Since I'm not using the most current release, I thought I'd ask about a behavior we are seeing that might be a potential bug.
>>> 
>>> It's  not the cause of your problem, but "event_add()"ing these events
>>> is nonsensical.  If an event has fd -1, it is meaningless (and
>>> undefined!) to give it EV_READ or EV_WRITE and assign.  It could cause
>>> bugs down the line if that's your standard practice.
>> 
>> Thanks - I'll correct that.
>> 
>>> 
>>> Looking at the issue, I think the problem occurs when you use
>>> event_active() to activate another event of priority equal to the
>>> currently executing event: since the event will happen in the
>>> currently executing instance of the event loop rather than the next
>>> one, it'll keep executing all events of that priority, including the
>>> new one, before it scans for new events and executes higher priority
>>> events again.
>>> 
>>> Or from another point of view, the problem is that using
>>> event_active() on a higher-priority event doesn't tell event to stop
>>> executing events at the current priority and go to the next event loop
>>> iteration.
>>> 
>>> Option 1:  Call this a bug. Say that if you use event_active() to
>>> activate an event of higher (that is, numerically lower) priority than
>>> the currently executing event, the event loop needs to check for new
>>> events and then execute higher-priority events.
>>> 
>>> Option 1a: apply the fix in 2.0, since it's probably right.
>>> Option 1b: apply the fix in 2.1, since it might potentially break
>>> something if anybody was relying on the old behavior.
>> 
>> I think either of these would be acceptable, and agree that this feels like a bug to me. If 2.1 can be released soon as other than alpha, I'm happy to bump up to that level.
> 
> Well, the 2.1 release schedule is what it is; every feature that goes
> in is an argument for doing it sooner, but every feature that isn't in
> yet is an argument for doing it later.  Current plan is to have it
> feature-freeze around the end of June and go into beta at that point,
> to be followed by -rc and -stable release when the bugs are shaken
> out.

Understood - perhaps it would be advisable to release a 2.0 patch in the interim, then.

> 
> That said, in case everybody _does_ decide that this is a 2.0 bug, how
> about a fix?  Have a look at branch "20_active_prio_inv" in my
> personal github repository[*].  It's against 2.0.  I'd appreciate a
> careful review to make sure it makes sense and works for everybody.

I'll take a look at your branch, but had some further thoughts in the interim. Would a more optimal option be to add the following logic to event_priority_set:

event_priority_set(ev, pri) {
    IF <I am in an event> AND
        IF <ev->base> EQ <current-base> AND
        IF <pri> LT <current-pri>  THEN
            <rescan queues on next loop>

In other words, only rescan and start the dispatch phase if the priority of the new event is higher (less-than) that of the current event. If not, then just proceed as we currently do. This avoid unnecessarily restarting the dispatch phase when lower priority events are defined.

> 
> [*]Incidentally, there are three Git repositories everybody who wants
> to follow Libevent development should know about.  There is the
> current official repository, which lives at
> git://levent.git.sourceforge.net/gitroot/levent/levent .  There is the
> future official repository, which lives at
> https://github.com/libevent/libevent.git .  And there's my personal
> repository, which lives at https://github.com/nmathewson/libevent.git
> .  The official repositories contain only official branches;
> everything there counts as "merged in."  My personal repository is
> where I stick work-in-progress, rough draft code, and so on.
> 
> 
> yrs,
> -- 
> Nick
> ***********************************************************************
> To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
> unsubscribe libevent-users    in the body.

***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.