On Thu, Jul 1, 2010 at 11:31 AM, xiaobing jiang <s7v7nislands@xxxxxxxxx> wrote:
> hi all:
> what's the usage of ev_pncalls in struct event? why not direct
> use ev_ncalls?
>
> two question:
> 1. in libevent 1.4.14, it seems only used in event_process_active(),
> but in libeven2 it used in event_signal_closure. why?
Because the ncalls logic really only makes sense for signals, so in
2.0.x it's made more signal-specific.
> 2. ev->ev_pncalls = &ncalls; // from event_process_active()
> here, ncalls is a stack variable.
>
> if (ev->ev_ncalls && ev->ev_pncalls) {
> /* Abort loop */
> *ev->ev_pncalls = 0; // ev.ev_pncalls point to a
> stack variable, will this cover the stack?
> }
>
A fine question. I think the answer is, "The code in 1.4.x is safe
and will not clobber the stack: if ncalls is nonzero, then we are
either inside event_process_active() where pncalls points to ncalls,
or we have called event_active, which sets pncalls to &ev->ev_ncalls."
Nevertheless, I'd feel a bit safer if we didn't leave the pointer to
an unused stack location lying around; what do you think of the
attached patch?
The idea behind ev_ncalls was to allow signal handlers to get run once
for each signal received. This is important for some programs,
especially ones that do things like treating one SIGINT as meaning
"shut down at your own convenience" and a second SIGINT as meaning
"shut down immediately."
The idea of the ev_pncalls pointer, as I understand it, was to provide
a good way for an event_del() called from inside an event callback for
a running signal event to safely delete that event. Once a callback
has called event_del, it's allowed to reassign that event, or free it,
or whatever. But this means that event_process_active() can't just
loop on ev->ev_ncalls, since any call to the callback might invalidate
ev->ev_ncalls. Instead, we loop on a stack variable, and the
ev_pncalls pointer gives the event_del() to modify that stack pointer.
But the 2.0.x situation is a little trickier. In fact, I think the
code in 2.0.x is not threadsafe wrt the ev_pncalls pointer. I've got
a bug report with a patch at
http://sourceforge.net/tracker/?func=detail&aid=3008667&group_id=50884&atid=461322
, and I'm really hoping somebody will have a chance to look it over
before I merge it.
peace,
--
Nick
Attachment:
0001-Clean-up-a-pointer-to-stack-in-event_process_active.patch
Description: Binary data