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)

9 files changed, 51 insertions, 16 deletions

diff --git a/src/feh.h b/src/feh.h
index 7088dea..1d48e53 100644
--- a/src/feh.h
+++ b/src/feh.h
@@ -188,4 +188,7 @@ extern feh_menu *menu_main;
 extern feh_menu *menu_close;
 extern char *mode;		/* label for the current mode */
 
+/* to terminate long-running children with SIGALRM */
+extern int childpid;
+
 #endif
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);
+			}
 		}
 	}
 
diff --git a/src/main.c b/src/main.c
index 5891056..a5694fb 100644
--- a/src/main.c
+++ b/src/main.c
@@ -30,6 +30,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include "timers.h"
 #include "options.h"
 #include "events.h"
+#include "signals.h"
 #include "wallpaper.h"
 
 char **cmdargv = NULL;
@@ -48,6 +49,7 @@ int main(int argc, char **argv)
 		init_x_and_imlib();
 		init_keyevents();
 		init_buttonbindings();
+		setup_signal_handlers();
 	}
 
 	feh_event_init();
diff --git a/src/multiwindow.c b/src/multiwindow.c
index 1bd8a8a..9797fb0 100644
--- a/src/multiwindow.c
+++ b/src/multiwindow.c
@@ -65,7 +65,5 @@ void init_multiwindow_mode(void)
 		free(s);
 	}
 
-	setup_signal_handlers();
-
 	return;
 }
diff --git a/src/options.c b/src/options.c
index f389e0e..a277003 100644
--- a/src/options.c
+++ b/src/options.c
@@ -53,6 +53,7 @@ void init_parse_options(int argc, char **argv)
 	opt.display = 1;
 	opt.aspect = 1;
 	opt.slideshow_delay = 0.0;
+	opt.magick_timeout = 10;
 	opt.thumb_w = 60;
 	opt.thumb_h = 60;
 	opt.thumb_redraw = 10;
@@ -378,6 +379,7 @@ static void feh_parse_option_array(int argc, char **argv, int finalrun)
 		{"zoom"          , 1, 0, 205},
 		{"no-screen-clip", 0, 0, 206},
 		{"index-info"    , 1, 0, 207},
+		{"magick-timeout", 1, 0, 208},
 		{"caption-path"  , 1, 0, 'K'},
 		{"action1"       , 1, 0, 209},
 		{"action2"       , 1, 0, 210},
@@ -646,6 +648,9 @@ static void feh_parse_option_array(int argc, char **argv, int finalrun)
 		case 207:
 			opt.index_info = estrdup(optarg);
 			break;
+		case 208:
+			opt.magick_timeout = atoi(optarg);
+			break;
 		case 'K':
 			opt.caption_path = estrdup(optarg);
 			break;
diff --git a/src/options.h b/src/options.h
index bee7c73..898e9ff 100644
--- a/src/options.h
+++ b/src/options.h
@@ -117,6 +117,8 @@ struct __fehoptions {
 
 	double slideshow_delay;
 
+	signed short magick_timeout;
+
 	Imlib_Font menu_fn;
 };
 
diff --git a/src/signals.c b/src/signals.c
index d5a6899..91f6bf3 100644
--- a/src/signals.c
+++ b/src/signals.c
@@ -36,9 +36,10 @@ void setup_signal_handlers()
 	if (
 		(sigemptyset(&feh_ss) == -1) ||
 		(sigaddset(&feh_ss, SIGUSR1) == -1) ||
-		(sigaddset(&feh_ss, SIGUSR2) == -1))
+		(sigaddset(&feh_ss, SIGUSR2) == -1) ||
+		(sigaddset(&feh_ss, SIGALRM) == -1))
 	{
-		weprintf("Failed to set up signal mask, SIGUSR1/2 won't work");
+		weprintf("Failed to set up signal masks");
 		return;
 	}
 
@@ -48,9 +49,10 @@ void setup_signal_handlers()
 
 	if (
 		(sigaction(SIGUSR1, &feh_sh, NULL) == -1) ||
-		(sigaction(SIGUSR2, &feh_sh, NULL) == -1))
+		(sigaction(SIGUSR2, &feh_sh, NULL) == -1) ||
+		(sigaction(SIGALRM, &feh_sh, NULL) == -1))
 	{
-		weprintf("Failed to set up signal handler, SIGUSR1/2 won't work");
+		weprintf("Failed to set up signal handler");
 		return;
 	}
 
@@ -59,10 +61,16 @@ void setup_signal_handlers()
 
 void feh_handle_signal(int signo)
 {
-	winwidget winwid
-		= winwidget_get_first_window_of_type(WIN_TYPE_SLIDESHOW);
+	winwidget winwid;
 	int i;
 
+	if (signo == SIGALRM) {
+		killpg(childpid, SIGINT);
+		kill(childpid, SIGINT);
+		return;
+	}
+	winwid = winwidget_get_first_window_of_type(WIN_TYPE_SLIDESHOW);
+
 	if (winwid) {
 		if (signo == SIGUSR1)
 			slideshow_change_image(winwid, SLIDE_NEXT, 1);
diff --git a/src/slideshow.c b/src/slideshow.c
index a868a44..0f67917 100644
--- a/src/slideshow.c
+++ b/src/slideshow.c
@@ -76,8 +76,6 @@ void init_slideshow_mode(void)
 	if (!success)
 		show_mini_usage();
 
-	setup_signal_handlers();
-
 	return;
 }
 
diff --git a/src/thumbnail.c b/src/thumbnail.c
index b13b3d8..f276592 100644
--- a/src/thumbnail.c
+++ b/src/thumbnail.c
@@ -32,6 +32,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include "md5.h"
 #include "feh_png.h"
 #include "index.h"
+#include "signals.h"
 
 static gib_list *thumbnails = NULL;