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 <[email protected]>
