On Apr 30, 2012, at 4:38 PM, Nick Mathewson wrote:
> On Mon, Apr 30, 2012 at 4:48 PM, Ralph Castain <r...@open-mpi.org> wrote:
>>
>> On Apr 30, 2012, at 2:30 PM, Nick Mathewson wrote:
>>
>>> On Mon, Apr 30, 2012 at 3:53 PM, Ralph Castain <r...@open-mpi.org> 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 majord...@freehaven.net with
> unsubscribe libevent-users in the body.
***********************************************************************
To unsubscribe, send an e-mail to majord...@freehaven.net with
unsubscribe libevent-users in the body.
