This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch master in repository libx2goclient. commit 54e91eed4cb4e0b9d602ae3509f4d9dd09f3197b Author: Mihai Moldovan <ionic@ionic.de> Date: Mon Aug 31 22:33:25 2020 +0200 src/x2goclient-network-ssh.c: move check timeout to a separate thread, add synchronization via a mutex. We cannot execute a timeout in the main (or, for that matter, callingg) thread, since that would require a running main loop. However, libx2goclient is supposed to be used by other applications and we cannot usefully force them into starting a glib main loop. Additionally, that would make executing any other code almost impossible, so that won't fly either way. Instead, spawn a different thread for this timer (timeout in glib-parlance) and synchronize access to instance variables (and, by extension, the newly added ones for the thread and the like). Also, make sure that the last check can run out correctly at object finalization time. --- src/x2goclient-network-ssh.c | 169 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 153 insertions(+), 16 deletions(-) diff --git a/src/x2goclient-network-ssh.c b/src/x2goclient-network-ssh.c index 56e1cd3..eb83c29 100644 --- a/src/x2goclient-network-ssh.c +++ b/src/x2goclient-network-ssh.c @@ -115,12 +115,21 @@ struct _X2GoClientNetworkSSH { char *control_path; gboolean active_master_conn; - guint check_timeout; + GSource *check_timeout_source; + GMutex check_thread_mutex; + GThread *check_thread; + GMainContext *check_thread_context; }; G_DEFINE_TYPE (X2GoClientNetworkSSH, x2goclient_network_ssh, X2GOCLIENT_TYPE_NETWORK); +struct x2goclient_network_ssh_check_timeout_data { + X2GoClientNetworkSSH *self; + GError **gerr; +}; + + /* See src/x2goclient-network.c regarding abbreviations. */ enum { X2GO_NET_SSH_PROP_OPENSSH_VERSION = 1, @@ -149,8 +158,12 @@ static gboolean x2goclient_network_ssh_gptrarray_to_string (GPtrArray * const ar static void x2goclient_network_ssh_gptrarray_print_debug (GPtrArray * const arr, const gchar * const prelude, const gchar * const desc); static gboolean x2goclient_network_ssh_sshcmd_add_host_port (X2GoClientNetworkSSH * const self, GPtrArray * const ssh_cmd, GError ** const gerr); static gboolean x2goclient_network_ssh_start_sshcmd (X2GoClientNetworkSSH * const self, const GPtrArray * const ssh_cmd, GError ** const gerr, const gboolean master); +static void x2goclient_network_ssh_check_timeout_data_free (struct x2goclient_network_ssh_check_timeout_data *data); +static gpointer x2goclient_network_ssh_check_timeout_thread_main (const gpointer user_data); static gboolean x2goclient_network_ssh_start_check_timeout (X2GoClientNetworkSSH * const self, GError ** const gerr); - +static void x2goclient_network_ssh_start_check_timeout_invoke (X2GoClientNetworkSSH * const self, GError ** const gerr); +static gboolean x2goclient_network_ssh_start_check_timeout_idle_unwrap (const gpointer user_data); +static gboolean x2goclient_network_ssh_start_check_timeout_real (X2GoClientNetworkSSH * const self, GError ** const gerr); static gboolean x2goclient_network_ssh_check_timeout (const gpointer self); @@ -190,7 +203,10 @@ static void x2goclient_network_ssh_init (X2GoClientNetworkSSH * const self) { self->control_path = NULL; self->active_master_conn = FALSE; - self->check_timeout = 0; + self->check_timeout_source = NULL; + g_mutex_init (&(self->check_thread_mutex)); + self->check_thread = NULL; + self->check_thread_context = NULL; } X2GoClientNetworkSSH* x2goclient_network_ssh_new (const char * const session_path) { @@ -235,12 +251,26 @@ static void x2goclient_network_ssh_finalize (GObject * const object) { * connection, so make sure that the dependencies are set up correctly here. */ - if (self->check_timeout) { - if (!(g_source_remove (self->check_timeout))) { - g_log (NULL, G_LOG_LEVEL_CRITICAL, "Unable to remove timeout with ID %u.", self->check_timeout); - } + /* Remove timeout source. */ + g_mutex_lock (&(self->check_thread_mutex)); + if (self->check_timeout_source) { + g_source_destroy (self->check_timeout_source); + g_source_unref (self->check_timeout_source); + self->check_timeout_source = NULL; } - self->check_timeout = 0; + g_mutex_unlock (&(self->check_thread_mutex)); + + /* Let last event run out and thread terminate. */ + (void) g_thread_join (self->check_thread); + self->check_thread = NULL; + + /* + * Context should be gone by now, implicitly through the thread's main + * method, so let's just get rid of the pointer. + */ + self->check_thread_context = NULL; + + g_mutex_clear (&(self->check_thread_mutex)); if (!(x2goclient_network_ssh_kill_subprocesses (self))) { g_log (NULL, G_LOG_LEVEL_CRITICAL, "Some subprocesses were not terminated correctly!"); @@ -726,7 +756,10 @@ static gboolean x2goclient_network_ssh_kill_subprocesses (X2GoClientNetworkSSH * g_return_val_if_fail (X2GOCLIENT_IS_NETWORK_SSH (self), ret); + g_mutex_lock (&(self->check_thread_mutex)); if (self->active_master_conn) { + g_mutex_unlock (&(self->check_thread_mutex)); + /* Cleanup, if necessary. */ g_log (NULL, G_LOG_LEVEL_DEBUG, "Master connection cleanup required."); @@ -735,7 +768,9 @@ static gboolean x2goclient_network_ssh_kill_subprocesses (X2GoClientNetworkSSH * /* Add control path options. */ g_ptr_array_add (ssh_cmd, g_strdup ("-o")); + g_mutex_lock (&(self->check_thread_mutex)); g_ptr_array_add (ssh_cmd, g_strdup_printf ("ControlPath=\"%s\"", self->control_path)); + g_mutex_unlock (&(self->check_thread_mutex)); /* Add master socket command. */ g_ptr_array_add (ssh_cmd, g_strdup ("-O")); @@ -752,9 +787,14 @@ static gboolean x2goclient_network_ssh_kill_subprocesses (X2GoClientNetworkSSH * g_log (NULL, G_LOG_LEVEL_DEBUG, "Launching!"); ret = x2goclient_network_ssh_start_sshcmd (self, ssh_cmd, NULL, FALSE); + g_mutex_lock (&(self->check_thread_mutex)); self->active_master_conn = (!(ret)); + g_mutex_unlock (&(self->check_thread_mutex)); } } + else { + g_mutex_unlock (&(self->check_thread_mutex)); + } return (ret); } @@ -797,7 +837,9 @@ static gboolean x2goclient_network_ssh_parent_connect (X2GoClientNetwork * const GString *tmp_string = g_string_new (session_path); g_string_append (tmp_string, "/ssh/control"); + g_mutex_lock (&(self->check_thread_mutex)); self->control_path = g_string_free (tmp_string, FALSE); + g_mutex_unlock (&(self->check_thread_mutex)); gchar *dirname = g_path_get_dirname (self->control_path); errno = 0; @@ -837,7 +879,9 @@ static gboolean x2goclient_network_ssh_parent_connect (X2GoClientNetwork * const g_log (NULL, G_LOG_LEVEL_INFO, "Fetched OpenSSH version: [ Major: %d, Minor: %d, Patch: %d, Additional Data: '%s' ]", self->openssh_version->major, self->openssh_version->minor, self->openssh_version->patch, self->openssh_version->addon); /* Update/populate bugs structure. */ + g_mutex_lock (&(self->check_thread_mutex)); ret = x2goclient_openssh_bugs_update (self->openssh_bugs, self->openssh_version, gerr); + g_mutex_unlock (&(self->check_thread_mutex)); } if (ret) { @@ -903,7 +947,10 @@ static gboolean x2goclient_network_ssh_parent_connect (X2GoClientNetwork * const x2goclient_network_ssh_gptrarray_print_debug (ssh_cmd, "Would try to connect via:", "OpenSSH client command"); g_log (NULL, G_LOG_LEVEL_DEBUG, "Launching!"); - self->active_master_conn = ret = x2goclient_network_ssh_start_sshcmd (self, ssh_cmd, gerr, TRUE); + ret = x2goclient_network_ssh_start_sshcmd (self, ssh_cmd, gerr, TRUE); + g_mutex_lock (&(self->check_thread_mutex)); + self->active_master_conn = ret; + g_mutex_unlock (&(self->check_thread_mutex)); } if (ret) { @@ -936,8 +983,10 @@ static gboolean x2goclient_network_ssh_fetch_openssh_version (X2GoClientNetworkS * it), so we'll have to duplicate a bit of code and can't use the * GObject property setter directly. */ + g_mutex_lock (&(self->check_thread_mutex)); x2goclient_openssh_version_free (self->openssh_version); self->openssh_version = version; + g_mutex_unlock (&(self->check_thread_mutex)); ret = TRUE; } @@ -1385,23 +1434,103 @@ static gboolean x2goclient_network_ssh_start_sshcmd (X2GoClientNetworkSSH * cons return (ret); } +static void x2goclient_network_ssh_check_timeout_data_free (struct x2goclient_network_ssh_check_timeout_data *data) { + /* No need to clear any data within the structure. */ + g_free (data); +} + +static gpointer x2goclient_network_ssh_check_timeout_thread_main (const gpointer user_data) { + gpointer ret = NULL; + + GMainContext *main_context = user_data; + GMainLoop *main_loop; + + /* Make given main context the default one for this thread. */ + g_main_context_push_thread_default (main_context); + + /* Add new main loop and execute it. */ + main_loop = g_main_loop_new (main_context, FALSE); + g_main_loop_run (main_loop); + + /* Clean up everything after g_main_loop_run () returned. */ + g_main_loop_unref (main_loop); + + g_main_context_pop_thread_default (main_context); + g_main_context_unref (main_context); + + return (ret); +} + static gboolean x2goclient_network_ssh_start_check_timeout (X2GoClientNetworkSSH * const self, GError ** const gerr) { gboolean ret = FALSE; g_return_val_if_fail (X2GOCLIENT_IS_NETWORK_SSH (self), ret); g_return_val_if_fail (((NULL == gerr) || (NULL == *gerr)), ret); - guint check_id = g_timeout_add (50, &x2goclient_network_ssh_check_timeout, self); + g_mutex_lock (&(self->check_thread_mutex)); + self->check_thread_context = g_main_context_new (); + g_mutex_unlock (&(self->check_thread_mutex)); + g_thread_new ("check_thread", &x2goclient_network_ssh_check_timeout_thread_main, g_main_context_ref (self->check_thread_context)); - ret = (!(!(check_id))); + x2goclient_network_ssh_start_check_timeout_invoke (self, gerr); - if (ret) { - self->check_timeout = check_id; + ret = TRUE; + + return (ret); +} + +static void x2goclient_network_ssh_start_check_timeout_invoke (X2GoClientNetworkSSH * const self, GError ** const gerr) { + g_return_if_fail (X2GOCLIENT_IS_NETWORK_SSH (self)); + g_return_if_fail (((NULL == gerr) || (NULL == *gerr))); + + struct x2goclient_network_ssh_check_timeout_data *data = g_new0 (struct x2goclient_network_ssh_check_timeout_data, 1); + data->self = self; + data->gerr = gerr; + + g_main_context_invoke_full (self->check_thread_context, G_PRIORITY_DEFAULT, + &x2goclient_network_ssh_start_check_timeout_idle_unwrap, + data, (GDestroyNotify) x2goclient_network_ssh_check_timeout_data_free); +} + +static gboolean x2goclient_network_ssh_start_check_timeout_idle_unwrap (const gpointer user_data) { + gboolean ret = G_SOURCE_CONTINUE; + + const struct x2goclient_network_ssh_check_timeout_data *data = user_data; + + g_return_val_if_fail (X2GOCLIENT_IS_NETWORK_SSH (data->self), ret); + g_return_val_if_fail (((NULL == data->gerr) || (NULL == *(data->gerr))), ret); + + if (x2goclient_network_ssh_start_check_timeout_real (data->self, data->gerr)) { + ret = G_SOURCE_REMOVE; } return (ret); } +static gboolean x2goclient_network_ssh_start_check_timeout_real (X2GoClientNetworkSSH * const self, GError ** const gerr) { + gboolean ret = FALSE; + + g_return_val_if_fail (X2GOCLIENT_IS_NETWORK_SSH (self), ret); + g_return_val_if_fail (((NULL == gerr) || (NULL == *gerr)), ret); + + g_mutex_lock (&(self->check_thread_mutex)); + + /* Add source. */ + self->check_timeout_source = g_timeout_source_new (50); + + /* Bind check function to timeout source. */ + g_source_set_callback (self->check_timeout_source, &x2goclient_network_ssh_check_timeout, self, NULL); + + /* Bind timeout source to thread context. */ + guint check_id = g_source_attach (self->check_timeout_source, self->check_thread_context); + + g_mutex_unlock (&(self->check_thread_mutex)); + + ret = (!(!(check_id))); + + return (ret); +} + static gboolean x2goclient_network_ssh_check_timeout (const gpointer data) { gboolean ret = FALSE; @@ -1414,7 +1543,9 @@ static gboolean x2goclient_network_ssh_check_timeout (const gpointer data) { /* Add control path options. */ g_ptr_array_add (ssh_cmd, g_strdup ("-o")); + g_mutex_lock (&(self->check_thread_mutex)); g_ptr_array_add (ssh_cmd, g_strdup_printf ("ControlPath=\"%s\"", self->control_path)); + g_mutex_unlock (&(self->check_thread_mutex)); /* Add master socket command. */ g_ptr_array_add (ssh_cmd, g_strdup ("-O")); @@ -1434,11 +1565,17 @@ static gboolean x2goclient_network_ssh_check_timeout (const gpointer data) { } if (!(ret)) { - /* We'll return FALSE, so make sure to reset the instance variable. */ - self->check_timeout = 0; + /* We'll return FALSE, so make sure to delete the source. */ + g_mutex_lock (&(self->check_thread_mutex)); + if (self->check_timeout_source) { + g_source_destroy (self->check_timeout_source); + g_source_unref (self->check_timeout_source); + self->check_timeout_source = NULL; + } - /* Also, mark the master connection is terminated/gone. */ + /* Also, mark the master connection as terminated/gone. */ self->active_master_conn = FALSE; + g_mutex_unlock (&(self->check_thread_mutex)); } return (ret); -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/libx2goclient.git