From 8f5bf736a9d3f8084c644dbf6ceefc4142715c4e Mon Sep 17 00:00:00 2001 From: Daniel Friesel Date: Tue, 13 Mar 2012 00:45:13 +0100 Subject: Experimental code to limit imagemagick convert runtime (see #82) Problems so far: * leaks zombie processes * does not work when terminating feh with a signal (since the convert process is no longer in feh's process group) --- src/imlib.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) (limited to 'src/imlib.c') diff --git a/src/imlib.c b/src/imlib.c index c0252c3..1e4eb3f 100644 --- a/src/imlib.c +++ b/src/imlib.c @@ -59,6 +59,8 @@ int xinerama_screen; int num_xinerama_screens; #endif /* HAVE_LIBXINERAMA */ +int childpid; + static char *feh_http_load_image(char *url); static char *feh_magick_load_image(char *filename); @@ -253,7 +255,10 @@ static char *feh_magick_load_image(char *filename) char *tmpname; char *sfn; int fd = -1, devnull = -1; - int pid, status; + int status; + + if (opt.magick_timeout < 0) + return NULL; basename = strrchr(filename, '/'); @@ -277,26 +282,39 @@ static char *feh_magick_load_image(char *filename) snprintf(argv_fd, sizeof(argv_fd), "png:fd:%d", fd); - - if ((pid = fork()) == 0) { + if ((childpid = fork()) == 0) { /* discard convert output */ devnull = open("/dev/null", O_WRONLY); dup2(devnull, 1); dup2(devnull, 2); + /* + * convert only accepts SIGINT via killpg, a normal kill doesn't work + */ + if (opt.magick_timeout) + setpgid(0, 0); + execlp("convert", "convert", filename, argv_fd, NULL); exit(1); } else { - waitpid(pid, &status, 0); + alarm(opt.magick_timeout); + waitpid(childpid, &status, 0); + alarm(0); if (!WIFEXITED(status) || (WEXITSTATUS(status) != 0)) { close(fd); unlink(sfn); sfn = NULL; - if (!opt.quiet) - weprintf("%s - No loader for that file format", filename); + if (!opt.quiet) { + if (WIFSIGNALED(status)) + weprintf("%s - Conversion took too long, skipping", + filename); + else + weprintf("%s - No loader for that file format", + filename); + } } } -- cgit v1.2.3 From 86a6ae59b3b2b63fd71c4668bfcf33d6fea8448b Mon Sep 17 00:00:00 2001 From: Daniel Friesel Date: Wed, 14 Mar 2012 13:09:02 +0100 Subject: imlib.c: "Fix" Zombie issue Still to do: signal handler for sigint/quit/term --- src/imlib.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'src/imlib.c') diff --git a/src/imlib.c b/src/imlib.c index 1e4eb3f..d83cc08 100644 --- a/src/imlib.c +++ b/src/imlib.c @@ -315,6 +315,17 @@ static char *feh_magick_load_image(char *filename) weprintf("%s - No loader for that file format", filename); } + + /* + * Reap child. The previous waitpid call was interrupted by + * alarm, but convert doesn't terminate immediately. + * XXX + * normally, if (WIFSIGNALED(status)) waitpid(childpid, &status, 0); + * would suffice. However, as soon as feh has its own window, + * this doesn't work anymore and the following workaround is + * required. Hm. + */ + waitpid(-1, &status, 0); } } -- cgit v1.2.3 From 1f4029b244eef2cd956447cae46ab5a194a8916c Mon Sep 17 00:00:00 2001 From: Daniel Friesel Date: Wed, 14 Mar 2012 13:34:48 +0100 Subject: Terminate convert when receiving SIG{INT,TERM,QUIT} --- src/imlib.c | 3 ++- src/signals.c | 26 ++++++++++++++++++++------ 2 files changed, 22 insertions(+), 7 deletions(-) (limited to 'src/imlib.c') diff --git a/src/imlib.c b/src/imlib.c index d83cc08..090159c 100644 --- a/src/imlib.c +++ b/src/imlib.c @@ -59,7 +59,7 @@ int xinerama_screen; int num_xinerama_screens; #endif /* HAVE_LIBXINERAMA */ -int childpid; +int childpid = 0; static char *feh_http_load_image(char *url); static char *feh_magick_load_image(char *filename); @@ -327,6 +327,7 @@ static char *feh_magick_load_image(char *filename) */ waitpid(-1, &status, 0); } + childpid = 0; } return sfn; diff --git a/src/signals.c b/src/signals.c index 91f6bf3..0b18aac 100644 --- a/src/signals.c +++ b/src/signals.c @@ -37,7 +37,10 @@ void setup_signal_handlers() (sigemptyset(&feh_ss) == -1) || (sigaddset(&feh_ss, SIGUSR1) == -1) || (sigaddset(&feh_ss, SIGUSR2) == -1) || - (sigaddset(&feh_ss, SIGALRM) == -1)) + (sigaddset(&feh_ss, SIGALRM) == -1) || + (sigaddset(&feh_ss, SIGTERM) == -1) || + (sigaddset(&feh_ss, SIGQUIT) == -1) || + (sigaddset(&feh_ss, SIGINT) == -1)) { weprintf("Failed to set up signal masks"); return; @@ -50,7 +53,10 @@ void setup_signal_handlers() if ( (sigaction(SIGUSR1, &feh_sh, NULL) == -1) || (sigaction(SIGUSR2, &feh_sh, NULL) == -1) || - (sigaction(SIGALRM, &feh_sh, NULL) == -1)) + (sigaction(SIGALRM, &feh_sh, NULL) == -1) || + (sigaction(SIGTERM, &feh_sh, NULL) == -1) || + (sigaction(SIGQUIT, &feh_sh, NULL) == -1) || + (sigaction(SIGINT, &feh_sh, NULL) == -1)) { weprintf("Failed to set up signal handler"); return; @@ -64,11 +70,19 @@ void feh_handle_signal(int signo) winwidget winwid; int i; - if (signo == SIGALRM) { - killpg(childpid, SIGINT); - kill(childpid, SIGINT); - return; + switch (signo) { + case SIGALRM: + if (childpid) + killpg(childpid, SIGINT); + return; + case SIGINT: + case SIGTERM: + case SIGQUIT: + if (childpid) + killpg(childpid, SIGINT); + exit(128 + signo); } + winwid = winwidget_get_first_window_of_type(WIN_TYPE_SLIDESHOW); if (winwid) { -- cgit v1.2.3 From 338553950063615b59d80d2148d208b490c3d0bd Mon Sep 17 00:00:00 2001 From: Daniel Friesel Date: Thu, 15 Mar 2012 21:07:50 +0100 Subject: feh_magick_load_image: close stdin as well --- src/imlib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/imlib.c') diff --git a/src/imlib.c b/src/imlib.c index 090159c..eda3e2a 100644 --- a/src/imlib.c +++ b/src/imlib.c @@ -286,14 +286,14 @@ static char *feh_magick_load_image(char *filename) /* discard convert output */ devnull = open("/dev/null", O_WRONLY); + dup2(devnull, 0); dup2(devnull, 1); dup2(devnull, 2); /* * convert only accepts SIGINT via killpg, a normal kill doesn't work */ - if (opt.magick_timeout) - setpgid(0, 0); + setpgid(0, 0); execlp("convert", "convert", filename, argv_fd, NULL); exit(1); -- cgit v1.2.3