[M] Change in libosmocore[master]: After writing VTY config, run sync() in a different thread

jolly has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email ) Change subject: After writing VTY config, run sync() in a different thread ...................................................................... After writing VTY config, run sync() in a different thread Running sync() after writing VTY config causes the process to stop for a fraction of a second. This may results in delayed processing that causes buffer underruns, overflows and delays. After writing VTY configuration to the config file, a new thread is created. This thread runs the sync() command. Once it returns, the thread will signal through a pipe to the main thread that the sync process is complete. The main process will then output the completion message on the VTY. Related: OS#6438 Change-Id: I3cb2ee68b2e4c730f96522208c4abf00d0f49a44 --- M src/vty/command.c 1 file changed, 87 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/98/37798/1 diff --git a/src/vty/command.c b/src/vty/command.c index 1719690..a6c5d94 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -39,6 +39,7 @@ #include <time.h> #include <limits.h> #include <signal.h> +#include <pthread.h> #include <sys/time.h> #include <sys/stat.h> #include <sys/types.h> @@ -51,6 +52,7 @@ #include <osmocom/core/talloc.h> #include <osmocom/core/timer.h> #include <osmocom/core/utils.h> +#include <osmocom/core/select.h> #ifndef MAXPATHLEN #define MAXPATHLEN 4096 @@ -3372,7 +3374,85 @@ return CMD_SUCCESS; } -static int write_config_file(const char *config_file, char **outpath) +static struct osmo_fd sync_fd; +static int sync_pipe_fd[2]; + +static int sync_complete(struct osmo_fd *fd, unsigned int what) +{ + struct vty *vty = fd->data; + + if (sync_pipe_fd[0]) + close(sync_pipe_fd[0]); + if (sync_pipe_fd[1]) + close(sync_pipe_fd[1]); + if (osmo_fd_is_registered(fd)) + osmo_fd_unregister(fd); + + /* Check if pointer to VTY session is still valid. + * If session closes before completion, prevent use-after-free condition. + */ + if (vty && vty_is_active(vty)) { + vty_out(vty, "%s", VTY_NEWLINE); + vty_out(vty, "Configuration saved to %s%s", host.config, VTY_NEWLINE); + } + + return 0; +} + +static void *sync_thread(void* arg) +{ + sync(); + write(sync_pipe_fd[1], "", 1); + + return NULL; +} + +/* Run sync thread and setup pipe to signal completion. */ +static void do_sync(struct vty *vty) +{ + int rc; + pthread_t thread; + + sync_pipe_fd[0] = 0; + sync_pipe_fd[1] = 0; + + if (osmo_fd_is_registered(&sync_fd)) { + LOGP(DLGLOBAL, LOGL_INFO, "There is another ongoing sync, syncing without thread.\n"); + sync(); + sync_complete(&sync_fd, 0); + return; + } + + rc = pipe(sync_pipe_fd); + if (rc < 0) { + LOGP(DLGLOBAL, LOGL_INFO, "Failed to create pipe, syncing without thread.\n"); + sync(); + sync_complete(&sync_fd, 0); + return; + } + + sync_fd.data = vty; + sync_fd.fd = sync_pipe_fd[0]; + sync_fd.when = OSMO_FD_READ; + sync_fd.cb = sync_complete; + rc = osmo_fd_register(&sync_fd); + if (rc < 0) { + LOGP(DLGLOBAL, LOGL_INFO, "Failed to register fd, syncing without thread.\n"); + sync(); + sync_complete(&sync_fd, 0); + return; + } + + rc = pthread_create(&thread, NULL, sync_thread, NULL); + if (rc < 0) { + LOGP(DLGLOBAL, LOGL_INFO, "Failed to create thread, syncing without thread.\n"); + sync(); + sync_complete(&sync_fd, 0); + return; + } +} + +static int write_config_file(struct vty *vty, const char *config_file, char **outpath) { unsigned int i; int fd; @@ -3451,12 +3531,12 @@ unlink(config_file_tmp); return -3; } - sync(); if (unlink(config_file) != 0) { *outpath = talloc_strdup(tall_vty_cmd_ctx, config_file); talloc_free(config_file_sav); talloc_free(config_file_tmp); unlink(config_file_tmp); + do_sync(NULL); return -4; } } @@ -3465,10 +3545,11 @@ talloc_free(config_file_sav); talloc_free(config_file_tmp); unlink(config_file_tmp); + do_sync(NULL); return -5; } unlink(config_file_tmp); - sync(); + do_sync(vty); talloc_free(config_file_sav); talloc_free(config_file_tmp); @@ -3511,7 +3592,7 @@ return CMD_WARNING; } - rc = write_config_file(host.config, &failed_file); + rc = write_config_file(vty, host.config, &failed_file); switch (rc) { case -1: vty_out(vty, "Can't open configuration file %s.%s", @@ -3544,7 +3625,6 @@ rc = CMD_WARNING; break; default: - vty_out(vty, "Configuration saved to %s%s", host.config, VTY_NEWLINE); rc = CMD_SUCCESS; break; } @@ -4377,7 +4457,7 @@ char *failed_file; int rc; - rc = write_config_file(filename, &failed_file); + rc = write_config_file(NULL, filename, &failed_file); talloc_free(failed_file); return rc; } @@ -4398,7 +4478,7 @@ if (host.config == NULL) return -7; - rc = write_config_file(host.config, &failed_file); + rc = write_config_file(NULL, host.config, &failed_file); talloc_free(failed_file); return rc; } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cb2ee68b2e4c730f96522208c4abf00d0f49a44 Gerrit-Change-Number: 37798 Gerrit-PatchSet: 1 Gerrit-Owner: jolly &lt;andreas(a)eversberg.eu&gt;