Fix fd_handlers when using recursive main loops.

If an fd_handler created a recursive main loop (just called
ecore_main_loop_begin()), then this recursive main loop should
continue to process fd_handlers from there and on, thus
fd_handler_current (and win32_handler_current) was added. When going
back from recursion, the current iterator should be updated properly.

This patch also fixes the deletion of fd_handler from recursive main
loops by reference counting them. This way, the node will not be
free()d inside inner loop cleanups and then crash when going back to
outer loop.

PS: win32 code is untested (or even compiled).

The following test case used to crash but not anymore:

#include <Ecore.h>
#include <Eina.h>
#include <unistd.h>

static int _log_dom;
#define INF(...) EINA_LOG_DOM_INFO(_log_dom, __VA_ARGS__)
#define ERR(...) EINA_LOG_DOM_ERR(_log_dom, __VA_ARGS__)

static Ecore_Fd_Handler *handle;
static int a[2], b[2];

static int cb2(void *data, Ecore_Fd_Handler *h)
{
    INF("cb2 - delete cb1 handle");
    ecore_main_fd_handler_del(handle);
    ecore_main_loop_quit(); /* quits inner main loop */
    return 0;
}

static int cb1(void *data, Ecore_Fd_Handler *h)
{
    unsigned char ch = 222;

    INF("cb1: begin");
    INF("    add cb2");
    ecore_main_fd_handler_add(b[0], ECORE_FD_READ, cb2, NULL, NULL, NULL);
    INF("    wake up pipe b");
    if (write(b[1], &ch, 1) != 1)
        ERR("could not write to pipe b");
    INF("    inner main loop begin (recurse)");
    ecore_main_loop_begin(); /* will it crash due
                              * ecore_main_fd_handler_del(handle)
                              * inside cb2()? It used to!
                              */
    INF("cb1: end");

    ecore_main_loop_quit(); /* quits outer main loop */

    return 0;
}

int main(void)
{
    unsigned char ch = 111;

    ecore_init();

    _log_dom = eina_log_domain_register("test", EINA_COLOR_CYAN);
    pipe(a);
    pipe(b);

    /*
     * Creating a new main loop from inside an fd_handler callback,
     * and inside this new (inner) main loop deleting the caller
     * callback used to crash since the handle would be effectively
     * free()d, but when the recursion is over the pointer would be
     * used.
     */

    INF("main: begin");
    handle = ecore_main_fd_handler_add
        (a[0], ECORE_FD_READ, cb1, NULL, NULL, NULL);
    INF("main: wake up pipe a");
    if (write(a[1], &ch, 1) != 1)
        ERR("could not write to pipe a");
    ecore_main_loop_begin();
    INF("main: end");
    return 0;
}



SVN revision: 46443
This commit is contained in:
Gustavo Sverzut Barbieri 2010-02-24 20:59:44 +00:00
parent 89594ec456
commit 2d60db1c2b
1 changed files with 100 additions and 32 deletions

View File

@ -54,6 +54,7 @@ struct _Ecore_Fd_Handler
void *buf_data;
void (*prep_func) (void *data, Ecore_Fd_Handler *fd_handler);
void *prep_data;
int references;
Eina_Bool read_active : 1;
Eina_Bool write_active : 1;
Eina_Bool error_active : 1;
@ -68,6 +69,7 @@ struct _Ecore_Win32_Handler
HANDLE h;
int (*func) (void *data, Ecore_Win32_Handler *win32_handler);
void *data;
int references;
Eina_Bool delete_me : 1;
};
#endif
@ -91,9 +93,11 @@ static void _ecore_main_win32_handlers_cleanup(void);
static int in_main_loop = 0;
static int do_quit = 0;
static Ecore_Fd_Handler *fd_handlers = NULL;
static Ecore_Fd_Handler *fd_handler_current = NULL;
static int fd_handlers_delete_me = 0;
#ifdef _WIN32
static Ecore_Win32_Handler *win32_handlers = NULL;
static Ecore_Win32_Handler *win32_handler_current = NULL;
static int win32_handlers_delete_me = 0;
#endif
@ -432,6 +436,7 @@ _ecore_main_shutdown(void)
free(fdh);
}
fd_handlers_delete_me = 0;
fd_handler_current = NULL;
#ifdef _WIN32
while (win32_handlers)
@ -445,6 +450,7 @@ _ecore_main_shutdown(void)
free(wh);
}
win32_handlers_delete_me = 0;
win32_handler_current = NULL;
#endif
}
@ -488,7 +494,11 @@ _ecore_main_select(double timeout)
/* call the prepare callback for all handlers */
EINA_INLIST_FOREACH(fd_handlers, fdh)
if (!fdh->delete_me && fdh->prep_func)
fdh->prep_func (fdh->prep_data, fdh);
{
fdh->references++;
fdh->prep_func (fdh->prep_data, fdh);
fdh->references--;
}
EINA_INLIST_FOREACH(fd_handlers, fdh)
if (!fdh->delete_me)
{
@ -562,12 +572,14 @@ _ecore_main_fd_handlers_bads_rem(void)
if (fdh->flags & ECORE_FD_ERROR)
{
ERR("Fd set for error! calling user");
if (!fdh->func(fdh->data, fdh))
{
ERR("Fd function err returned 0, remove it");
fdh->delete_me = 1;
fd_handlers_delete_me = 1;
}
fdh->references++;
if (!fdh->func(fdh->data, fdh))
{
ERR("Fd function err returned 0, remove it");
fdh->delete_me = 1;
fd_handlers_delete_me = 1;
}
fdh->references--;
}
else
{
@ -587,6 +599,7 @@ _ecore_main_fd_handlers_cleanup(void)
{
Ecore_Fd_Handler *fdh;
Eina_Inlist *l;
int deleted_in_use = 0;
if (!fd_handlers_delete_me) return;
for (l = EINA_INLIST_GET(fd_handlers); l; )
@ -597,13 +610,21 @@ _ecore_main_fd_handlers_cleanup(void)
if (fdh->delete_me)
{
// ERR("Removing fd %d", fdh->fd);
if (fdh->references)
{
deleted_in_use++;
continue;
}
fd_handlers = (Ecore_Fd_Handler *) eina_inlist_remove(EINA_INLIST_GET(fd_handlers),
EINA_INLIST_GET(fdh));
ECORE_MAGIC_SET(fdh, ECORE_MAGIC_NONE);
free(fdh);
}
}
fd_handlers_delete_me = 0;
if (!deleted_in_use)
fd_handlers_delete_me = 0;
}
#ifdef _WIN32
@ -612,6 +633,7 @@ _ecore_main_win32_handlers_cleanup(void)
{
Ecore_Win32_Handler *wh;
Eina_Inlist *l;
int deleted_in_use = 0;
if (!win32_handlers_delete_me) return;
for (l = EINA_INLIST_GET(win32_handlers); l; )
@ -621,38 +643,64 @@ _ecore_main_win32_handlers_cleanup(void)
l = l->next;
if (wh->delete_me)
{
if (wh->references)
{
deleted_in_use++;
continue;
}
win32_handlers = (Ecore_Win32_Handler *)eina_inlist_remove(EINA_INLIST_GET(win32_handlers),
EINA_INLIST_GET(wh));
ECORE_MAGIC_SET(wh, ECORE_MAGIC_NONE);
free(wh);
}
}
win32_handlers_delete_me = 0;
if (!deleted_in_use)
win32_handlers_delete_me = 0;
}
#endif
static void
_ecore_main_fd_handlers_call(void)
{
Ecore_Fd_Handler *fdh;
if (!fd_handler_current)
{
/* regular main loop, start from head */
fd_handler_current = fd_handlers;
}
else
{
/* recursive main loop, continue from where we were */
fd_handler_current = (Ecore_Fd_Handler *)EINA_INLIST_GET(fd_handler_current)->next;
}
EINA_INLIST_FOREACH(fd_handlers, fdh)
if (!fdh->delete_me)
{
if ((fdh->read_active) ||
(fdh->write_active) ||
(fdh->error_active))
{
if (!fdh->func(fdh->data, fdh))
{
fdh->delete_me = 1;
fd_handlers_delete_me = 1;
}
fdh->read_active = 0;
while (fd_handler_current)
{
Ecore_Fd_Handler *fdh = fd_handler_current;
if (!fdh->delete_me)
{
if ((fdh->read_active) ||
(fdh->write_active) ||
(fdh->error_active))
{
fdh->references++;
if (!fdh->func(fdh->data, fdh))
{
fdh->delete_me = 1;
fd_handlers_delete_me = 1;
}
fdh->references--;
fdh->read_active = 0;
fdh->write_active = 0;
fdh->error_active = 0;
}
}
}
}
if (fd_handler_current) /* may have changed in recursive main loops */
fd_handler_current = (Ecore_Fd_Handler *)EINA_INLIST_GET(fd_handler_current)->next;
}
}
static int
@ -667,11 +715,13 @@ _ecore_main_fd_handlers_buf_call(void)
{
if (fdh->buf_func)
{
fdh->references++;
if (fdh->buf_func(fdh->buf_data, fdh))
{
ret |= fdh->func(fdh->data, fdh);
fdh->read_active = 1;
}
fdh->references--;
}
}
@ -957,15 +1007,33 @@ _ecore_main_win32_select(int nfds, fd_set *readfds, fd_set *writefds,
}
else if ((result >= WAIT_OBJECT_0 + events_nbr) && (result < WAIT_OBJECT_0 + objects_nbr))
{
EINA_INLIST_FOREACH(win32_handlers, wh)
{
if (!win32_handler_current)
{
/* regular main loop, start from head */
win32_handler_current = win32_handlers;
}
else
{
/* recursive main loop, continue from where we were */
win32_handler_current = (Ecore_Win32_Handler *)EINA_INLIST_GET(win32_handler_current)->next;
}
while (win32_handler_current)
{
wh = win32_handler_current;
if (objects[result - WAIT_OBJECT_0] == wh->h)
if (!wh->delete_me)
if (!wh->func(wh->data, wh))
{
wh->delete_me = 1;
win32_handlers_delete_me = 1;
}
{
wh->references++;
if (!wh->func(wh->data, wh))
{
wh->delete_me = 1;
win32_handlers_delete_me = 1;
}
wh->references--;
}
}
res = 1;
}