[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 1/3] server - Use a GLib main loop in the main thread
From: |
Jeremy Whiting |
Subject: |
[PATCH 1/3] server - Use a GLib main loop in the main thread |
Date: |
Wed, 29 Jul 2015 18:31:25 -0600 |
Looks good to me and runs well too. Ship it! :)
On Wed, Jul 29, 2015 at 6:15 PM, Luke Yelavich
<luke.yelavich at canonical.com> wrote:
> From: Luke Yelavich <themuso at themuso.com>
>
> This lays the groundwork for further enhancements and features.
> A bonus is simplified code, and probing only the file descriptors we care
> about.
>
> Also terminate on SIGTERM and SIGKILL signals.
> ---
> configure.ac | 2 +-
> src/server/speechd.c | 231
> +++++++++++++++++++++++++++------------------------
> src/server/speechd.h | 9 +-
> 3 files changed, 125 insertions(+), 117 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index da49a6f..0956534 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -35,7 +35,7 @@ AC_SEARCH_LIBS([pthread_create], [pthread], [],
> AC_SEARCH_LIBS([lt_dlopen], [ltdl], [],
> [AC_MSG_FAILURE([ltdl library missing])])
>
> -PKG_CHECK_MODULES([GLIB], [glib-2.0 >= 2.32])
> +PKG_CHECK_MODULES([GLIB], [glib-2.0 >= 2.36])
> AC_SUBST([GLIB_CFLAGS])
> AC_SUBST([GLIB_LIBS])
>
> diff --git a/src/server/speechd.c b/src/server/speechd.c
> index 77104ff..be5efcb 100644
> --- a/src/server/speechd.c
> +++ b/src/server/speechd.c
> @@ -58,7 +58,20 @@ void destroy_pid_file();
> /* Server socket file descriptor */
> int server_socket;
>
> -void speechd_load_configuration(int sig);
> +GMainLoop *main_loop = NULL;
> +
> +static gboolean speechd_client_terminate(gpointer key, gpointer value,
> gpointer user);
> +static gboolean speechd_reload_dead_modules(gpointer user_data);
> +static gboolean speechd_load_configuration(gpointer user_data);
> +static gboolean speechd_quit(gpointer user_data);
> +
> +static gboolean server_process_incoming (gint fd,
> + GIOCondition condition,
> + gpointer data);
> +
> +static gboolean client_process_incoming (gint fd,
> + GIOCondition condition,
> + gpointer data);
>
> #ifdef __SUNPRO_C
> /* Added by Willie Walker - daemon is a gcc-ism
> @@ -349,7 +362,6 @@ int speechd_connection_new(int server_socket)
> }
>
> /* We add the associated client_socket to the descriptor set. */
> - FD_SET(client_socket, &readfds);
> if (client_socket > SpeechdStatus.max_fd)
> SpeechdStatus.max_fd = client_socket;
> MSG(4, "Adding client on fd %d", client_socket);
> @@ -363,7 +375,6 @@ int speechd_connection_new(int server_socket)
> "Error: Failed to create a record in fd_settings for the
> new client");
> if (SpeechdStatus.max_fd == client_socket)
> SpeechdStatus.max_fd--;
> - FD_CLR(client_socket, &readfds);
> return -1;
> }
> new_fd_set->fd = client_socket;
> @@ -378,8 +389,11 @@ int speechd_connection_new(int server_socket)
> g_hash_table_insert(fd_settings, p_client_uid, new_fd_set);
> g_hash_table_insert(fd_uid, p_client_socket, p_client_uid2);
>
> + new_fd_set->fd_source = g_unix_fd_add(client_socket, G_IO_IN,
> client_process_incoming, NULL);
> +
> MSG(4, "Data structures for client on fd %d created", client_socket);
> return 0;
> +
> }
>
> int speechd_connection_destroy(int fd)
> @@ -394,6 +408,7 @@ int speechd_connection_destroy(int fd)
> if (fdset_element != NULL) {
> fdset_element->fd = -1;
> fdset_element->active = 0;
> + g_source_remove(fdset_element->fd_source);
> /* The fdset_element will be freed and removed from the
> hash table as soon as the client no longer has any
> message in the queues, check out the speak() function */
> @@ -413,7 +428,6 @@ int speechd_connection_destroy(int fd)
> if (SPEECHD_DEBUG)
> DIE("Can't close file descriptor associated to this
> client");
>
> - FD_CLR(fd, &readfds);
> if (fd == SpeechdStatus.max_fd)
> SpeechdStatus.max_fd--;
>
> @@ -422,7 +436,7 @@ int speechd_connection_destroy(int fd)
> return 0;
> }
>
> -gboolean speechd_client_terminate(gpointer key, gpointer value, gpointer
> user)
> +static gboolean speechd_client_terminate(gpointer key, gpointer value,
> gpointer user)
> {
> TFDSetElement *set;
>
> @@ -504,13 +518,14 @@ void speechd_module_nodebug(gpointer data, gpointer
> user_data)
> return;
> }
>
> -void speechd_reload_dead_modules(int sig)
> +static gboolean speechd_reload_dead_modules(gpointer user_data)
> {
> /* Reload dead modules */
> g_list_foreach(output_modules, speechd_modules_reload, NULL);
>
> /* Make sure there aren't any more child processes left */
> while (waitpid(-1, NULL, WNOHANG) > 0) ;
> + return TRUE;
> }
>
> void speechd_modules_debug(void)
> @@ -628,7 +643,7 @@ void speechd_init()
> /* Load configuration from the config file */
> MSG(4, "Reading Speech Dispatcher configuration from %s",
> SpeechdOptions.conf_file);
> - speechd_load_configuration(0);
> + speechd_load_configuration(NULL);
>
> logging_init();
>
> @@ -644,7 +659,7 @@ void speechd_init()
> last_p5_block = NULL;
> }
>
> -void speechd_load_configuration(int sig)
> +static gboolean speechd_load_configuration(gpointer user_data)
> {
> configfile_t *configfile = NULL;
> GList *detected_modules = NULL;
> @@ -706,47 +721,14 @@ void speechd_load_configuration(int sig)
> }
>
> free_config_options(spd_options, &spd_num_options);
> +
> + return TRUE;
> }
>
> -void speechd_quit(int sig)
> +static gboolean speechd_quit(gpointer user_data)
> {
> - int ret;
> -
> - MSG(1, "Terminating...");
> -
> - MSG(2, "Closing open connections...");
> - /* We will browse through all the connections and close them. */
> - g_hash_table_foreach_remove(fd_settings, speechd_client_terminate,
> - NULL);
> - g_hash_table_destroy(fd_settings);
> -
> - MSG(4, "Closing speak() thread...");
> - ret = pthread_cancel(speak_thread);
> - if (ret != 0)
> - FATAL("Speak thread failed to cancel!\n");
> -
> - ret = pthread_join(speak_thread, NULL);
> - if (ret != 0)
> - FATAL("Speak thread failed to join!\n");
> -
> - MSG(2, "Closing open output modules...");
> - /* Call the close() function of each registered output module. */
> - g_list_foreach(output_modules, speechd_modules_terminate, NULL);
> - g_list_free(output_modules);
> -
> - MSG(2, "Closing server connection...");
> - if (close(server_socket) == -1)
> - MSG(2, "close() failed: %s", strerror(errno));
> - FD_CLR(server_socket, &readfds);
> -
> - MSG(4, "Removing pid file");
> - destroy_pid_file();
> -
> - fflush(NULL);
> -
> - MSG(2, "Speech Dispatcher terminated correctly");
> -
> - exit(0);
> + g_main_loop_quit(main_loop);
> + return FALSE;
> }
>
> /* --- PID FILES --- */
> @@ -920,12 +902,53 @@ int make_inet_socket(const int port)
> return server_socket;
> }
>
> +gboolean server_process_incoming (gint fd,
> + GIOCondition condition,
> + gpointer data)
> +{
> + int ret;
> +
> + ret = speechd_connection_new(fd);
> + if (ret != 0) {
> + MSG(2, "Error: Failed to add new client!");
> + if (SPEECHD_DEBUG) {
> + FATAL("Failed to add new client");
> + }
> + }
> +
> + return TRUE;
> +}
> +
> +gboolean client_process_incoming (gint fd,
> + GIOCondition condition,
> + gpointer data)
> +{
> + int ret;
> + int nread;
> +
> + ioctl(fd, FIONREAD, &nread);
> +
> + if (nread == 0) {
> + /* client has gone */
> + ret = speechd_connection_destroy(fd);
> + if (ret != 0) {
> + MSG(2, "Error: Failed to close the client!");
> + }
> + return FALSE;
> + }
> +
> + /* client sends some commands or data */
> + if (serve(fd) == -1) {
> + MSG(2, "Error: Failed to serve client on fd %d!", fd);
> + }
> +
> + return TRUE;
> +}
> +
> /* --- MAIN --- */
>
> int main(int argc, char *argv[])
> {
> - fd_set testfds;
> - int fd;
> int ret;
> /* Autospawn helper variables */
> char *spawn_communication_method = NULL;
> @@ -1066,12 +1089,6 @@ int main(int argc, char *argv[])
> exit(1);
> }
>
> - /* Register signals */
> - (void)signal(SIGINT, speechd_quit);
> - (void)signal(SIGHUP, speechd_load_configuration);
> - (void)signal(SIGPIPE, SIG_IGN);
> - (void)signal(SIGUSR1, speechd_reload_dead_modules);
> -
> speechd_init();
>
> /* Handle socket_path 'default' */
> @@ -1185,70 +1202,66 @@ int main(int argc, char *argv[])
> return -1;
> }
>
> + /* Set up the main loop and register signals */
> + main_loop = g_main_loop_new(g_main_context_default(), FALSE);
> + g_unix_signal_add(SIGINT, speechd_quit, NULL);
> + g_unix_signal_add(SIGTERM, speechd_quit, NULL);
> + g_unix_signal_add(SIGHUP, speechd_load_configuration, NULL);
> + g_unix_signal_add(SIGUSR1, speechd_reload_dead_modules, NULL);
> + (void)signal(SIGPIPE, SIG_IGN);
> +
> MSG(4, "Creating new thread for speak()");
> ret = pthread_create(&speak_thread, NULL, speak, NULL);
> if (ret != 0)
> FATAL("Speak thread failed!\n");
>
> - FD_ZERO(&readfds);
> - FD_SET(server_socket, &readfds);
> SpeechdStatus.max_fd = server_socket;
>
> + g_unix_fd_add(server_socket, G_IO_IN,
> + server_process_incoming, NULL);
> +
> /* Now wait for clients and requests. */
> MSG(1, "Speech Dispatcher started and waiting for clients ...");
> - while (1) {
> - testfds = readfds;
> -
> - if (select
> - (FD_SETSIZE, &testfds, (fd_set *) 0, (fd_set *) 0,
> - NULL) >= 1) {
> - /* Once we know we've got activity,
> - * we find which descriptor it's on by checking each
> in turn using FD_ISSET.
> - TODO: This is a hack. We should not be browsing all
> possible socket numbers
> - but only those which are connected.
> - */
> - for (fd = 0;
> - fd <= SpeechdStatus.max_fd && fd < FD_SETSIZE;
> - fd++) {
> - if (FD_ISSET(fd, &testfds)) {
> - MSG(4, "Activity on fd %d ...", fd);
> -
> - if (fd == server_socket) {
> - /* server activity (new
> client) */
> - ret =
> - speechd_connection_new
> - (server_socket);
> - if (ret != 0) {
> - MSG(2,
> - "Error: Failed to
> add new client!");
> - if (SPEECHD_DEBUG)
> - FATAL
> - ("Failed
> to add new client");
> - }
> - } else {
> - /* client activity */
> - int nread;
> - ioctl(fd, FIONREAD, &nread);
> -
> - if (nread == 0) {
> - /* client has gone */
> -
> speechd_connection_destroy
> - (fd);
> - if (ret != 0)
> - MSG(2,
> - "Error:
> Failed to close the client!");
> - } else {
> - /* client sends some
> commands or data */
> - if (serve(fd) == -1)
> - MSG(2,
> - "Error:
> Failed to serve client on fd %d!",
> - fd);
> - }
> - }
> - }
> - }
> - }
> - }
> +
> + g_main_loop_run(main_loop);
> +
> + MSG(1, "Terminating...");
> +
> + MSG(2, "Closing open connections...");
> + /* We will browse through all the connections and close them. */
> + g_hash_table_foreach_remove(fd_settings, speechd_client_terminate,
> + NULL);
> + g_hash_table_destroy(fd_settings);
> +
> + MSG(4, "Closing speak() thread...");
> + ret = pthread_cancel(speak_thread);
> + if (ret != 0)
> + FATAL("Speak thread failed to cancel!\n");
> +
> + ret = pthread_join(speak_thread, NULL);
> + if (ret != 0)
> + FATAL("Speak thread failed to join!\n");
> +
> + MSG(2, "Closing open output modules...");
> + /* Call the close() function of each registered output module. */
> + g_list_foreach(output_modules, speechd_modules_terminate, NULL);
> + g_list_free(output_modules);
> +
> + MSG(2, "Closing server connection...");
> + if (close(server_socket) == -1)
> + MSG(2, "close() failed: %s", strerror(errno));
> +
> + MSG(4, "Removing pid file");
> + destroy_pid_file();
> +
> + fflush(NULL);
> +
> + g_main_loop_unref(main_loop);
> + main_loop = NULL;
> +
> + MSG(2, "Speech Dispatcher terminated correctly");
> +
> + exit(0);
> }
>
> void check_locked(pthread_mutex_t * lock)
> diff --git a/src/server/speechd.h b/src/server/speechd.h
> index a1066e4..1f6de7d 100644
> --- a/src/server/speechd.h
> +++ b/src/server/speechd.h
> @@ -45,6 +45,7 @@
> #include <pthread.h>
>
> #include <glib.h>
> +#include <glib-unix.h>
>
> #include <semaphore.h>
> #include <sys/ipc.h>
> @@ -75,6 +76,7 @@ union semun {
> typedef struct {
> unsigned int uid; /* Unique ID of the client */
> int fd; /* File descriptor the client is on. */
> + guint fd_source; /* Used to store the GSource ID for watching
> fd activity in the main loop */
> int active; /* Is this client still active on socket or
> gone? */
> int paused; /* Internal flag, 1 for paused client or 0
> for normal. */
> int paused_while_speaking;
> @@ -210,9 +212,6 @@ GList *last_p5_block;
> /* Global default settings */
> TFDSetElement GlobalFDSet;
>
> -/* Variables for socket communication */
> -fd_set readfds;
> -
> /* Inter thread comm pipe */
> int speaking_pipe[2];
>
> @@ -258,17 +257,13 @@ char *spd_get_path(char *filename, char *startdir);
> /* Functions used in speechd.c only */
> int speechd_connection_new(int server_socket);
> int speechd_connection_destroy(int fd);
> -gboolean speechd_client_terminate(gpointer key, gpointer value, gpointer
> user);
> void speechd_modules_terminate(gpointer data, gpointer user_data);
> void speechd_modules_reload(gpointer data, gpointer user_data);
> void speechd_modules_debug(void);
> void speechd_modules_nodebug(void);
>
> -void speechd_reload_dead_modules(int sig);
> void speechd_options_init(void);
> void speechd_init(void);
> -void speechd_load_configuration(int sig);
> -void speechd_quit(int sig);
> int create_pid_file(void);
> void destroy_pid_file(void);
>
> --
> 2.4.6
>
>
> _______________________________________________
> Speechd mailing list
> Speechd at lists.freebsoft.org
> http://lists.freebsoft.org/mailman/listinfo/speechd