clean up signal handling using a polling method, from the genius that is mackstann

git-svn-id: https://svn.musicpd.org/mpd/trunk@697 09075e82-0dd4-0310-85a5-a0d7c8717e4f

src/Makefile.am

@@ -5,13 +5,13 @@ mpd_headers = buffer2array.h interface.h command.h playlist.h ls.h \
 	tag.h player.h listen.h conf.h ogg_decode.h volume.h flac_decode.h \
 	audio.h playerData.h stats.h myfprintf.h sig_handlers.h decode.h log.h \
 	audiofile_decode.h charConv.h permission.h mpd_types.h pcm_utils.h \
-	mp4_decode.h aac_decode.h
+	mp4_decode.h aac_decode.h signal_check.h
 mpd_SOURCES = main.c buffer2array.c interface.c command.c playlist.c ls.c \
 	song.c list.c directory.c tables.c utils.c path.c mp3_decode.c \
 	tag.c player.c listen.c conf.c ogg_decode.c volume.c flac_decode.c \
 	audio.c playerData.c stats.c myfprintf.c sig_handlers.c decode.c log.c \
 	audiofile_decode.c charConv.c permission.c pcm_utils.c mp4_decode.c \
-	aac_decode.c $(mpd_headers)
+	aac_decode.c signal_check.c $(mpd_headers)
 
 mpd_CFLAGS = $(MPD_CFLAGS)
 mpd_LDADD = $(MPD_LIBS) $(ID3_LIB) $(MAD_LIB) $(MP4FF_LIB)

src/decode.c

@@ -58,9 +58,10 @@ void decodeSigHandler(int sig) {
 	if(sig==SIGCHLD) {
 		int status;
 		if(decode_pid && *decode_pid==wait3(&status,WNOHANG,NULL)) {
-			if(WIFSIGNALED(status) && WTERMSIG(status)!=SIGTERM) {
+			if(WIFSIGNALED(status) && WTERMSIG(status)!=SIGTERM &&
-				ERROR("decode process died from a "
+					WTERMSIG(status)!=SIGINT) 
-						"non-TERM signal: %i\n",
+			{
+				ERROR("decode process died from signal: %i\n",
 						WTERMSIG(status));
 			}
 			*decode_pid = 0;

src/directory.c

@@ -74,6 +74,8 @@ char directorydb[MAXPATHLEN+1];
 
 int directory_updatePid = 0;
 
+int directory_reReadDB = 0;
+
 mpd_uint16 directory_updateJobId = 0;
 
 DirectoryList * newDirectoryList();
@@ -112,16 +114,22 @@ void directory_sigChldHandler(int pid, int status) {
                                         WTERMSIG(status));
                 }
 		else if(WEXITSTATUS(status)==EXIT_SUCCESS) {
-			readDirectoryDB();
-			incrPlaylistVersion();
 			DEBUG("direcotry_sigChldHandler: "
 					"updated db succesffully\n");
+			directory_reReadDB = 1;
 		}
-		else ERROR("problems updating db\n");
 		directory_updatePid = 0;
 	}
 }
 
+void readDirectoryDBIfUpdateIsFinished() {
+	if(directory_reReadDB && 0==directory_updatePid) {
+		DEBUG("readDirectoryDB since update finished successfully\n");
+		readDirectoryDB();
+		directory_reReadDB = 0;
+	}
+}
+
 int updateInit(FILE * fp, List * pathList) {
 	if(directory_updatePid > 0) {
 		myfprintf(fp,"%s already updating\n",COMMAND_RESPOND_ERROR);
@@ -135,13 +143,6 @@ int updateInit(FILE * fp, List * pathList) {
        	if(directory_updatePid==0) {
 		unblockSignals();
               	/* child */
-               	struct sigaction sa;
-               	sa.sa_flags = 0;
-               	sigemptyset(&sa.sa_mask);
-
-               	sa.sa_handler = SIG_IGN;
-               	sigaction(SIGPIPE,&sa,NULL);
-               	sigaction(SIGCHLD,&sa,NULL);
 
 		finishSigHandlers();
                	close(listenSocket);

src/directory.h

@@ -29,6 +29,8 @@
 
 extern char directorydb[MAXPATHLEN+1];
 
+void readDirectoryDBIfUpdateIsFinished();
+
 int isUpdatingDB();
 
 void directory_sigChldHandler(int pid, int status);

src/main.c

@@ -451,10 +451,10 @@ int main(int argc, char * argv[]) {
         while(COMMAND_RETURN_KILL!=doIOForInterfaces()) {
                 syncPlayerAndPlaylist();
                 closeOldInterfaces();
+		if(COMMAND_RETURN_KILL==handlePendingSignals()) break;
+		readDirectoryDBIfUpdateIsFinished();
         }
 
-        finishSigHandlers();
-
         savePlaylistState();
         playerKill();
 

src/player.c

@@ -69,9 +69,11 @@ void resetPlayer() {
 
 void player_sigChldHandler(int pid, int status) {
 	if(player_pid==pid) {
-		if(WIFSIGNALED(status) && WTERMSIG(status)!=SIGTERM) {
+		DEBUG("SIGCHLD caused by player process\n");
-			ERROR("player process died from a "
+		if(WIFSIGNALED(status) && WTERMSIG(status)!=SIGTERM &&
-					"non-TERM signal: %i\n",
+				WTERMSIG(status)!=SIGINT) 
+		{
+			ERROR("player process died from signal: %i\n",
 					WTERMSIG(status));
 		}
 		resetPlayer();
@@ -100,12 +102,11 @@ int playerInit() {
 		sa.sa_flags = 0;
 		sigemptyset(&sa.sa_mask);
 
-		sa.sa_handler = SIG_IGN;
+		finishSigHandlers();
-		sigaction(SIGPIPE,&sa,NULL);
-		sigaction(SIGHUP,&sa,NULL);
 		sa.sa_handler = decodeSigHandler;
 		sigaction(SIGCHLD,&sa,NULL);
 		sigaction(SIGTERM,&sa,NULL);
+		sigaction(SIGINT,&sa,NULL);
 
 		close(listenSocket);
 		freeAllInterfaces();

src/playlist.c

@@ -102,8 +102,6 @@ void initPlaylist() {
 	playlist.random = 0;
 	playlist.queued = -1;
 
-	blockTermSignal();
-
 	playlist_max_length = strtol((getConf())[CONF_MAX_PLAYLIST_LENGTH],&test,10);
 	if(*test!='\0') {
 		ERROR("max playlist length \"%s\" is not an integer\n",
@@ -137,7 +135,6 @@ void initPlaylist() {
 		playlist_stateFile = getConf()[CONF_STATE_FILE];
 	}
 
-	unblockTermSignal();
 }
 
 void finishPlaylist() {
@@ -152,10 +149,8 @@ int clearPlaylist(FILE * fp) {
 
 	if(stopPlaylist(fp)<0) return -1;
 
-	blockTermSignal();
 	for(i=0;i<playlist.length;i++) playlist.songs[i] = NULL;
 	playlist.length = 0;
-	unblockTermSignal();
 
 	incrPlaylistVersion();
 
@@ -176,7 +171,6 @@ void savePlaylistState() {
 	if(playlist_stateFile) {
 		FILE * fp;
 
-		blockTermSignal();
 		while(!(fp = fopen(playlist_stateFile,"w")) && errno==EINTR);
 		if(!fp) {
 			ERROR("problems opening state file \"%s\" for "
@@ -216,7 +210,6 @@ void savePlaylistState() {
 		myfprintf(fp,"%s\n",PLAYLIST_STATE_FILE_PLAYLIST_END);
 
 		while(fclose(fp) && errno==EINTR);
-		unblockTermSignal();
 	}
 }
 
@@ -518,7 +511,6 @@ int swapSongsInPlaylist(FILE * fp, int song1, int song2) {
 		return -1;
 	}
 
-	blockTermSignal();
 	
 	if(playlist_state==PLAYLIST_STATE_PLAY) {
 		if(playlist.queued>=0) {
@@ -552,8 +544,6 @@ int swapSongsInPlaylist(FILE * fp, int song1, int song2) {
 		else if(playlist.current==song2) playlist.current = song1;
 	}
 
-	unblockTermSignal();
-
 	incrPlaylistVersion();
 
 	return 0;
@@ -572,7 +562,6 @@ int deleteFromPlaylist(FILE * fp, int song) {
 		return -1;
 	}
 
-	blockTermSignal();
 	if(playlist_state==PLAYLIST_STATE_PLAY) {
 		if(playlist.queued>=0 && (playlist.order[playlist.queued]==song
 			|| playlist.order[playlist.current]==song)) 
@@ -602,8 +591,6 @@ int deleteFromPlaylist(FILE * fp, int song) {
 	playlist.songs[playlist.length-1] = NULL;
 	playlist.length--;
 
-	unblockTermSignal();
-
 	incrPlaylistVersion();
 
 	if(playlist_state!=PLAYLIST_STATE_STOP && playlist.current==songOrder) {
@@ -820,7 +807,6 @@ int moveSongInPlaylist(FILE * fp, int from, int to) {
 		return -1;
 	}
 
-	blockTermSignal();
 	
 	if(playlist_state==PLAYLIST_STATE_PLAY) {
 		if(playlist.queued>=0) {
@@ -866,8 +852,6 @@ int moveSongInPlaylist(FILE * fp, int from, int to) {
 		playlist.current++;
 	}
 
-	unblockTermSignal();
-
 	incrPlaylistVersion();
 
 	return 0;
@@ -878,7 +862,6 @@ void orderPlaylist() {
 
 	playlist.current = playlist.order[playlist.current];
 
-	blockTermSignal();
 	if(playlist_state==PLAYLIST_STATE_PLAY) {
 		if(playlist.queued>=0) {
 			lockPlaylistInteraction();
@@ -891,7 +874,6 @@ void orderPlaylist() {
 		playlist.order[i] = i;
 	}
 
-	unblockTermSignal();
 }
 
 void swapOrder(int a, int b) {
@@ -906,8 +888,6 @@ void randomizeOrder(int start,int end) {
 
 	DEBUG("playlist: randomize from %i to %i\n",start,end);
 
-	blockTermSignal();
-
 	if(playlist_state==PLAYLIST_STATE_PLAY) {
 		if(playlist.queued>=start && playlist.queued<=end) {
 			lockPlaylistInteraction();
@@ -923,7 +903,6 @@ void randomizeOrder(int start,int end) {
 		swapOrder(i,ri);
 	}
 
-	unblockTermSignal();
 }
 
 int setPlaylistRandomStatus(FILE * fp, int status) {
@@ -978,7 +957,6 @@ int shufflePlaylist(FILE * fp) {
 	int ri;
 
 	if(playlist.length>1) {
-		blockTermSignal();
 		if(playlist_state==PLAYLIST_STATE_PLAY) {
 			lockPlaylistInteraction();
 			clearPlayerQueue();
@@ -999,7 +977,6 @@ int shufflePlaylist(FILE * fp) {
 			ri = rand()%(playlist.length-1)+1;
 			swapSongs(i,ri);
 		}
-		unblockTermSignal();
 
 		incrPlaylistVersion();
 	}
@@ -1068,7 +1045,6 @@ int savePlaylist(FILE * fp, char * utf8file) {
 		return -1;
 	}
 
-
 	while(!(fileP = fopen(actualFile,"w")) && errno==EINTR);
 	if(fileP==NULL) {
 		myfprintf(fp,"%s Problems opening file\n",COMMAND_RESPOND_ERROR);

src/sig_handlers.c

@@ -20,35 +20,41 @@
 #include "player.h"
 #include "playlist.h"
 #include "directory.h"
+#include "command.h"
+#include "signal_check.h"
+#include "log.h"
 
 #include <signal.h>
 #include <sys/types.h>
 #include <sys/time.h>
 #include <sys/resource.h>
 #include <sys/wait.h>
+#include <errno.h>
 
-struct sigaction original_termSa;
+int handlePendingSignals() {
-struct sigaction original_hupSa;
+        if(signal_is_pending(SIGINT) || signal_is_pending(SIGTERM)) {
+                DEBUG("got SIGINT or SIGTERM, exiting\n");
+		return COMMAND_RETURN_KILL;
+        }
 
-void termSigHandler(int signal) {
+	if(signal_is_pending(SIGHUP)) {
-	if(signal==SIGTERM) {
+                DEBUG("got SIGHUP, rereading DB\n");
-		savePlaylistState();
+		signal_clear(SIGHUP);
-		playerKill();
+		readDirectoryDB();
-		exit(EXIT_SUCCESS);
 	}
-}
 
-void usr1SigHandler(int signal) {
+	return 0;
-}
-
-void hupSigHandler(int signal) {
-	readDirectoryDB();
 }
 
 void chldSigHandler(int signal) {
 	int status;
 	int pid;
-	while((pid = wait3(&status,WNOHANG,NULL)) > 0) {
+	DEBUG("got SIGCHLD\n");
+	while(0 != (pid = wait3(&status,WNOHANG,NULL))) {
+		if(pid<0) {
+			if(errno==EINTR) continue;
+			else break;
+		}
 		player_sigChldHandler(pid,status);
 		directory_sigChldHandler(pid,status);
 	}
@@ -61,19 +67,19 @@ void initSigHandlers() {
 	sigemptyset(&sa.sa_mask);
 	sa.sa_handler = SIG_IGN;
 	sigaction(SIGPIPE,&sa,NULL);
-	sa.sa_handler = usr1SigHandler;
-	sigaction(SIGUSR1,&sa,NULL);
 	sa.sa_handler = chldSigHandler;
 	sigaction(SIGCHLD,&sa,NULL);
-	sa.sa_handler = hupSigHandler;
+        signal_handle(SIGUSR1);
-	sigaction(SIGHUP,&sa,&original_hupSa);
+        signal_handle(SIGINT);
-	sa.sa_handler = termSigHandler;
+        signal_handle(SIGTERM);
-	sigaction(SIGTERM,&sa,&original_termSa);
+        signal_handle(SIGHUP);
 }
 
 void finishSigHandlers() {
-	sigaction(SIGHUP,&original_termSa,NULL);
+        signal_unhandle(SIGINT);
-	sigaction(SIGTERM,&original_termSa,NULL);
+        signal_unhandle(SIGUSR1);
+        signal_unhandle(SIGTERM);
+        signal_unhandle(SIGHUP);
 }
 
 void blockSignals() {
@@ -95,21 +101,3 @@ void unblockSignals() {
 	sigaddset(&sset,SIGHUP);
 	sigprocmask(SIG_UNBLOCK,&sset,NULL);
 }
-
-void blockTermSignal() {
-	sigset_t sset;
-
-	sigemptyset(&sset);
-	sigaddset(&sset,SIGTERM);
-	sigaddset(&sset,SIGHUP);
-	sigprocmask(SIG_BLOCK,&sset,NULL);
-}
-
-void unblockTermSignal() {
-	sigset_t sset;
-
-	sigemptyset(&sset);
-	sigaddset(&sset,SIGTERM);
-	sigaddset(&sset,SIGHUP);
-	sigprocmask(SIG_UNBLOCK,&sset,NULL);
-}

src/sig_handlers.h

@@ -21,6 +21,8 @@
 
 #include "../config.h"
 
+int handlePendingSignals();
+
 void initSigHandlers();
 
 void finishSigHandlers();

src/signal_check.c

@@ -0,0 +1,38 @@
+#include "signal_check.h"
+
+volatile sig_atomic_t __caught_signals[NSIG];
+
+static void __signal_handler(int sig)
+{
+        __caught_signals[sig] = 1;
+}
+
+static void __set_signal_handler(int sig, void (* handler)(int))
+{
+        struct sigaction act;
+        act.sa_flags = 0;
+        act.sa_handler = handler;
+        sigaction(sig, &act, 0);
+}
+
+void signal_handle(int sig)
+{
+        __set_signal_handler(sig, __signal_handler);
+}
+
+void signal_unhandle(int sig)
+{
+        signal_clear(sig);
+        __set_signal_handler(sig, SIG_DFL);
+}
+
+int signal_is_pending(int sig)
+{
+        return __caught_signals[sig];
+}
+
+void signal_clear(int sig)
+{
+        __caught_signals[sig] = 0;
+}
+

src/signal_check.h

@@ -0,0 +1,12 @@
+#ifndef SIGNAL_CHECK_H
+#define SIGNAL_CHECK_H
+
+#include <signal.h>
+
+void signal_handle(int sig);
+void signal_unhandle(int sig);
+int signal_is_pending(int sig);
+void signal_clear(int sig);
+
+#endif /* SIGNAL_CHECK_H */
+