[ WARNING: Long and ardorous - difficult to trace behaviour ]

Behaviour described is without EV_PERSIST on internal signal handler (as it's
been for months):

1. signal comes in, we jump to our internal signal handler, it writes to pipe.
2. epoll_wait() returns -1/EINTR because a signal handler was called.
3. we do not process epoll events here, we call evsignal_process() and return 
from dispatch.
4. our user-side event is deleted from queues, marked active, and processed 
normally BY evsignal_process().
5. user-side event then adds itself back normally to handle signals again.
6. we head back into epoll_dispatch(), where epoll_wait() immediately returns 
because
signal pipe is ready for reading (from step 1).
7. we implicitly delete the internal signal event (remember, this is without 
EV_PERSIST on it) and
mark is as active. however, since it is an internal event, we do not increment 
event_count_active.
8. we return to our main event loop within event.c, and do not call 
event_process_active() because
event_count_active == 0. this is where the problems really start.
9. we head back into epoll_dispatch() and wait normally. however, since we 
previously deleted
our internal signal pipe fd from the epoll set, and did NOT call it from 
event_process_active() in
the previous step - the pipe never ends up being read. but since the user 
callback was executed
properly, everything looks "normal."
10. we send the same signal again.  epoll_wait() returns, and we call 
evsignal_process() again.
11. here's the funny part: evsignal_process() increments event_count_active - 
but since our
internal signal event is actually first on the active queue at this point, when 
we go to
process events in the main loop - we actually end up processing our internal 
signal
event here. it reads from the pipe, and then re-adds itself back into epoll's 
watch set.
but since it's internal, we do not decrement event_count_active - guaranteeing 
we'll
get to our user event when we process the active events from the main loop. 
this is
the "flip-flop".
12. repeat.

Now with EV_PERSIST on the internal signal event, steps 7-12 do not happen the 
first
time the signal is called. So not only is the bug still there, it's super bad 
when
it happens as epoll_wait() returns immediately. By sending a signal again - we 
can flip
it from spin to wait.

So this is a bug that needs to be fixed anyways. The EV_PERSIST change just
made it more visible. I'm not so sure how to make a regression test for this 
other
than a specific test to make sure that our signal pipe is empty AFTER returning 
from
event_process_active().

--
test case:
#include <signal.h>
#include <stdlib.h>
#include <unistd.h>
#include <event.h>

void scb(int sig, short event, void *a)
{
        write(2, ".\n", 2);
        event_add(a, NULL);

        return;
}

int main(void)
{
        struct event sig;

        event_init();
        event_set(&sig, SIGTSTP, EV_SIGNAL, scb, &sig);
        event_add(&sig, NULL);
        event_dispatch();

        return 0;
}


--
debug log (unpatched):
$ ./sigtest
[debug] event_add: event: 0x7fff762c0500,    call 0x400768
[debug] event_queue_insert: 0x7fff762c0500: docount == 1, event_count == 1, 
queue == 4
[debug] evsignal_add: evsignal > sh_old_max, s == 20, max == 0
[debug] event_add: event: 0x602050, EV_READ   call 0x2ac034a166d4
[debug] event_queue_insert: 0x602050: docount == 0, event_count == 1, queue == 2

<wait, signal>

[debug] evsignal_handler: wake up
[debug] epoll_dispatch: epoll_wait() res == -1, errno = 4
[debug] evsignal_process: processing signals, caught == 1
[debug] event_del: 0x7fff762c0500, callback 0x400768
[debug] evsignal_del: 0x7fff762c0500: restoring previous handler
[debug] event_active: 0x7fff762c0500: adding to active queue, ev_res == 0, res 
== 8
[debug] event_queue_insert: 0x7fff762c0500: docount == 1, event_count == 1, 
queue == 8
[debug] event_base_loop: event_count_active == 1
.
[debug] event_add: event: 0x7fff762c0500,    call 0x400768
[debug] event_queue_insert: 0x7fff762c0500: docount == 1, event_count == 1, 
queue == 4
[debug] epoll_dispatch: epoll_wait reports 1
[debug] event_del: 0x602050, callback 0x2ac034a166d4
[debug] event_active: 0x602050: adding to active queue, ev_res == 0, res == 2
[debug] event_queue_insert: 0x602050: docount == 0, event_count == 1, queue == 8
[debug] event_base_loop: event_count_active == 0


<wait, signal>
(notice how we immediately call our internal event from event_base_loop?)


[debug] evsignal_handler: wake up
[debug] epoll_dispatch: epoll_wait() res == -1, errno = 4
[debug] evsignal_process: processing signals, caught == 1
[debug] event_del: 0x7fff762c0500, callback 0x400768
[debug] evsignal_del: 0x7fff762c0500: restoring previous handler
[debug] event_active: 0x7fff762c0500: adding to active queue, ev_res == 8, res 
== 8
[debug] event_queue_insert: 0x7fff762c0500: docount == 1, event_count == 1, 
queue == 8
[debug] event_base_loop: event_count_active == 1
[debug] evsignal_cb: n == 2
                    ^^^^^^^^

[debug] event_add: event: 0x602050, EV_READ   call 0x2ac034a166d4
[debug] event_queue_insert: 0x602050: docount == 0, event_count == 1, queue == 2
.
[debug] event_add: event: 0x7fff762c0500,    call 0x400768
[debug] event_queue_insert: 0x7fff762c0500: docount == 1, event_count == 1, 
queue == 4


--
debug log (patched):
$ ./sigtest
[debug] event_add: event: 0x7fff05e0b050,    call 0x400768
[debug] event_queue_insert: 0x7fff05e0b050: docount == 1, event_count == 1, 
queue == 4
[debug] evsignal_add: evsignal > sh_old_max, s == 20, max == 0
[debug] event_add: event: 0x602050, EV_READ   call 0x2b07a4ece6c4
[debug] event_queue_insert: 0x602050: docount == 0, event_count == 1, queue == 2

<wait, signal>

[debug] evsignal_handler: wake up
[debug] epoll_dispatch: epoll_wait() res == -1, errno = 4
[debug] evsignal_process: processing signals, caught == 1
[debug] event_del: 0x7fff05e0b050, callback 0x400768
[debug] evsignal_del: 0x7fff05e0b050: restoring previous handler
[debug] event_active: 0x7fff05e0b050: adding to active queue, ev_res == 0, res 
== 8
[debug] event_queue_insert: 0x7fff05e0b050: docount == 1, event_count == 1, 
queue == 8
[debug] event_base_loop: event_count_active == 1
.
[debug] event_add: event: 0x7fff05e0b050,    call 0x400768
[debug] event_queue_insert: 0x7fff05e0b050: docount == 1, event_count == 1, 
queue == 4
[debug] epoll_dispatch: epoll_wait reports 1
[debug] event_active: 0x602050: adding to active queue, ev_res == 0, res == 2
[debug] event_queue_insert: 0x602050: docount == 0, event_count == 1, queue == 8
[debug] event_base_loop: event_count_active == 1
[debug] evsignal_cb: n == 1

<wait, signal>

[debug] evsignal_handler: wake up
[debug] epoll_dispatch: epoll_wait() res == -1, errno = 4
[debug] evsignal_process: processing signals, caught == 1
[debug] event_del: 0x7fff05e0b050, callback 0x400768
[debug] evsignal_del: 0x7fff05e0b050: restoring previous handler
[debug] event_active: 0x7fff05e0b050: adding to active queue, ev_res == 8, res 
== 8
[debug] event_queue_insert: 0x7fff05e0b050: docount == 1, event_count == 1, 
queue == 8
[debug] event_base_loop: event_count_active == 1
.
[debug] event_add: event: 0x7fff05e0b050,    call 0x400768
[debug] event_queue_insert: 0x7fff05e0b050: docount == 1, event_count == 1, 
queue == 4
[debug] epoll_dispatch: epoll_wait reports 1
[debug] event_active: 0x602050: adding to active queue, ev_res == 2, res == 2
[debug] event_queue_insert: 0x602050: docount == 0, event_count == 1, queue == 8
[debug] event_base_loop: event_count_active == 1
[debug] evsignal_cb: n == 1

<repeats consistently>

--
patch:

Index: event.c
===================================================================
--- event.c     (revision 507)
+++ event.c     (working copy)
@@ -844,8 +844,7 @@
        ev->ev_flags &= ~queue;
        switch (queue) {
        case EVLIST_ACTIVE:
-               if (docount)
-                       base->event_count_active--;
+               base->event_count_active--;
                TAILQ_REMOVE(base->activequeues[ev->ev_pri],
                    ev, ev_active_next);
                break;
@@ -886,8 +885,7 @@
        ev->ev_flags |= queue;
        switch (queue) {
        case EVLIST_ACTIVE:
-               if (docount)
-                       base->event_count_active++;
+               base->event_count_active++;
                TAILQ_INSERT_TAIL(base->activequeues[ev->ev_pri],
                    ev,ev_active_next);
                break;

_______________________________________________
Libevent-users mailing list
Libevent-users@monkey.org
http://monkeymail.org/mailman/listinfo/libevent-users

Reply via email to