This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
laforge gerrit-no-reply at lists.osmocom.orglaforge has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/libosmocore/+/13312 )
Change subject: context: Add support for [per-thread] global talloc contexts
......................................................................
context: Add support for [per-thread] global talloc contexts
Rather than having applications maintain their own talloc cotexts,
let's offer some root talloc contexts in libosmocore. Let's also
make them per thread right from the beginning. This will help
some multi-threaded applications to use talloc in a thread-safe
way.
Change-Id: Iae39cd57274bf6753ecaf186f229e582b42662e3
---
M include/osmocom/core/select.h
M include/osmocom/core/talloc.h
M src/Makefile.am
A src/context.c
M src/select.c
M tests/Makefile.am
A tests/context/context_test.c
A tests/context/context_test.ok
M tests/testsuite.at
9 files changed, 213 insertions(+), 9 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/include/osmocom/core/select.h b/include/osmocom/core/select.h
index e4787b0..a200b6f 100644
--- a/include/osmocom/core/select.h
+++ b/include/osmocom/core/select.h
@@ -51,6 +51,7 @@
void osmo_fd_unregister(struct osmo_fd *fd);
void osmo_fd_close(struct osmo_fd *fd);
int osmo_select_main(int polling);
+int osmo_select_main_ctx(int polling);
struct osmo_fd *osmo_fd_get_by_fd(int fd);
diff --git a/include/osmocom/core/talloc.h b/include/osmocom/core/talloc.h
index 191a463..c68a56c 100644
--- a/include/osmocom/core/talloc.h
+++ b/include/osmocom/core/talloc.h
@@ -1,5 +1,27 @@
-/*! \file talloc.h
- * Convenience wrapper. libosmocore used to ship its own internal copy of
- * talloc, before libtalloc became a standard component on most systems */
+/*! \file talloc.h */
#pragma once
#include <talloc.h>
+
+/*! per-thread talloc contexts. This works around the problem that talloc is not
+ * thread-safe. However, one can simply have a different set of talloc contexts for each
+ * thread, and ensure that allocations made on one thread are always only free'd on that
+ * very same thread.
+ * WARNING: Users must make sure they free() on the same thread as they allocate!! */
+struct osmo_talloc_contexts {
+ /*! global per-thread talloc context. */
+ void *global;
+ /*! volatile select-dispatch context. This context is completely free'd and
+ * re-created every time the main select loop in osmo_select_main() returns from
+ * select(2) and calls per-fd callback functions. This allows users of this
+ * facility to allocate temporary objects like string buffers, message buffers
+ * and the like which are automatically free'd when going into the next select()
+ * system call */
+ void *select;
+};
+
+extern __thread struct osmo_talloc_contexts *osmo_ctx;
+
+/* short-hand #defines for the osmo talloc contexts (OTC) that can be used to pass
+ * to the various _c functions like msgb_alloc_c() */
+#define OTC_GLOBAL (osmo_ctx->global)
+#define OTC_SELECT (osmo_ctx->select)
diff --git a/src/Makefile.am b/src/Makefile.am
index f937810..245eb6d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -13,7 +13,7 @@
lib_LTLIBRARIES = libosmocore.la
libosmocore_la_LIBADD = $(BACKTRACE_LIB) $(TALLOC_LIBS) $(LIBRARY_RT)
-libosmocore_la_SOURCES = timer.c timer_gettimeofday.c timer_clockgettime.c \
+libosmocore_la_SOURCES = context.c timer.c timer_gettimeofday.c timer_clockgettime.c \
select.c signal.c msgb.c bits.c \
bitvec.c bitcomp.c counter.c fsm.c \
write_queue.c utils.c socket.c \
diff --git a/src/context.c b/src/context.c
new file mode 100644
index 0000000..bad012b
--- /dev/null
+++ b/src/context.c
@@ -0,0 +1,52 @@
+/*! \file context.c
+ * talloc context handling.
+ *
+ * (C) 2019 by Harald Welte <laforge at gnumonks.org>
+ * All Rights Reserverd.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+#include <string.h>
+#include <errno.h>
+#include <osmocom/core/talloc.h>
+#include <osmocom/core/utils.h>
+
+__thread struct osmo_talloc_contexts *osmo_ctx;
+
+int osmo_ctx_init(const char *id)
+{
+ osmo_ctx = talloc_named(NULL, sizeof(*osmo_ctx), "global-%s", id);
+ if (!osmo_ctx)
+ return -ENOMEM;
+ memset(osmo_ctx, 0, sizeof(*osmo_ctx));
+ osmo_ctx->global = osmo_ctx;
+ osmo_ctx->select = talloc_named_const(osmo_ctx->global, 0, "select");
+ if (!osmo_ctx->select) {
+ talloc_free(osmo_ctx);
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+/* initialize osmo_ctx on main tread */
+static __attribute__((constructor)) void on_dso_load_ctx(void)
+{
+ OSMO_ASSERT(osmo_ctx_init("main") == 0);
+}
+
+/*! @} */
diff --git a/src/select.c b/src/select.c
index 7ce135f..394a59d 100644
--- a/src/select.c
+++ b/src/select.c
@@ -36,6 +36,8 @@
#include <osmocom/core/linuxlist.h>
#include <osmocom/core/timer.h>
#include <osmocom/core/logging.h>
+#include <osmocom/core/talloc.h>
+#include <osmocom/core/utils.h>
#include "../config.h"
@@ -233,11 +235,7 @@
return work;
}
-/*! select main loop integration
- * \param[in] polling should we pollonly (1) or block on select (0)
- * \returns 0 if no fd handled; 1 if fd handled; negative in case of error
- */
-int osmo_select_main(int polling)
+static int _osmo_select_main(int polling)
{
fd_set readset, writeset, exceptset;
int rc;
@@ -259,10 +257,42 @@
/* fire timers */
osmo_timers_update();
+ OSMO_ASSERT(osmo_ctx->select);
+
/* call registered callback functions */
return osmo_fd_disp_fds(&readset, &writeset, &exceptset);
}
+/*! select main loop integration
+ * \param[in] polling should we pollonly (1) or block on select (0)
+ * \returns 0 if no fd handled; 1 if fd handled; negative in case of error
+ */
+int osmo_select_main(int polling)
+{
+ int rc = _osmo_select_main(polling);
+#ifndef EMBEDDED
+ if (talloc_total_size(osmo_ctx->select) != 0) {
+ osmo_panic("You cannot use the 'select' volatile "
+ "context if you don't use osmo_select_main_ctx()!\n");
+ }
+#endif
+ return rc;
+}
+
+#ifndef EMBEDDED
+/*! select main loop integration with temporary select-dispatch talloc context
+ * \param[in] polling should we pollonly (1) or block on select (0)
+ * \returns 0 if no fd handled; 1 if fd handled; negative in case of error
+ */
+int osmo_select_main_ctx(int polling)
+{
+ int rc = _osmo_select_main(polling);
+ /* free all the children of the volatile 'select' scope context */
+ talloc_free_children(osmo_ctx->select);
+ return rc;
+}
+#endif
+
/*! find an osmo_fd based on the integer fd
* \param[in] fd file descriptor to use as search key
* \returns \ref osmo_fd for \ref fd; NULL in case it doesn't exist */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a8a06c5..92edf75 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -32,6 +32,7 @@
tdef/tdef_vty_test_dynamic \
sockaddr_str/sockaddr_str_test \
use_count/use_count_test \
+ context/context_test \
$(NULL)
if ENABLE_MSGFILE
@@ -251,6 +252,9 @@
use_count_use_count_test_SOURCES = use_count/use_count_test.c
use_count_use_count_test_LDADD = $(LDADD)
+context_context_test_SOURCES = context/context_test.c
+context_context_test_LDADD = $(LDADD)
+
# The `:;' works around a Bash 3.2 bug when the output is not writeable.
$(srcdir)/package.m4: $(top_srcdir)/configure.ac
:;{ \
@@ -322,6 +326,7 @@
tdef/tdef_vty_test_dynamic.vty \
sockaddr_str/sockaddr_str_test.ok \
use_count/use_count_test.ok use_count/use_count_test.err \
+ context/context_test.ok \
$(NULL)
DISTCLEANFILES = atconfig atlocal conv/gsm0503_test_vectors.c
diff --git a/tests/context/context_test.c b/tests/context/context_test.c
new file mode 100644
index 0000000..e9a5559
--- /dev/null
+++ b/tests/context/context_test.c
@@ -0,0 +1,84 @@
+#include <stdint.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+
+#include <sys/eventfd.h>
+
+#include <osmocom/core/logging.h>
+#include <osmocom/core/application.h>
+#include <osmocom/core/utils.h>
+#include <osmocom/core/select.h>
+#include <osmocom/core/talloc.h>
+
+static struct osmo_fd g_evfd;
+
+static void *alloc_res_select;
+static void *alloc_res_global;
+
+static int destructor_called;
+
+static int talloc_destructor(void *ptr)
+{
+ printf("destructor was called automatically\n");
+ /* ensure the destructor is only called for the chunk allocated from the
+ * volatile select context */
+ OSMO_ASSERT(ptr == alloc_res_select);
+ destructor_called += 1;
+ return 0;
+}
+
+static int evfd_cb(struct osmo_fd *ofd, unsigned int what)
+{
+ uint64_t rval;
+ int rc;
+
+ rc = read(ofd->fd, &rval, sizeof(rval));
+ OSMO_ASSERT(rc == sizeof(rval));
+
+ printf("allocating from select context\n");
+ alloc_res_select = talloc_named_const(OTC_SELECT, 23, "alloc_select");
+ OSMO_ASSERT(alloc_res_select);
+ talloc_set_destructor(alloc_res_select, talloc_destructor);
+
+ printf("allocating from global context\n");
+ alloc_res_global = talloc_named_const(OTC_GLOBAL, 42, "alloc_global");
+ OSMO_ASSERT(alloc_res_global);
+ talloc_set_destructor(alloc_res_global, talloc_destructor);
+ return 0;
+}
+
+const struct log_info_cat default_categories[] = {
+};
+
+static struct log_info info = {
+ .cat = default_categories,
+ .num_cat = ARRAY_SIZE(default_categories),
+};
+
+int main(int argc, char **argv)
+{
+ int rc;
+
+ osmo_init_logging2(OTC_GLOBAL, &info);
+
+ rc = eventfd(0, 0);
+ OSMO_ASSERT(rc >= 0);
+ osmo_fd_setup(&g_evfd, rc, OSMO_FD_READ, evfd_cb, NULL, 0);
+ osmo_fd_register(&g_evfd);
+
+ /* make sure the select loop will immediately call the callback */
+ uint64_t val = 1;
+ rc = write(g_evfd.fd, &val, sizeof(val));
+ OSMO_ASSERT(rc == sizeof(val));
+
+ /* enter osmo_select_main_ctx() once */
+ printf("entering osmo_select_main\n");
+ osmo_select_main_ctx(1);
+
+ /* the allocation must have happened, and the destructor must have been called
+ * automatically exactly once */
+ OSMO_ASSERT(destructor_called == 1);
+
+ exit(0);
+}
diff --git a/tests/context/context_test.ok b/tests/context/context_test.ok
new file mode 100644
index 0000000..3984a39
--- /dev/null
+++ b/tests/context/context_test.ok
@@ -0,0 +1,4 @@
+entering osmo_select_main
+allocating from select context
+allocating from global context
+destructor was called automatically
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 0fc9646..a043f0c 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -350,3 +350,9 @@
cat $abs_srcdir/use_count/use_count_test.err > experr
AT_CHECK([$abs_top_builddir/tests/use_count/use_count_test], [0], [expout], [experr])
AT_CLEANUP
+
+AT_SETUP([context])
+AT_KEYWORDS([context])
+cat $abs_srcdir/context/context_test.ok > expout
+AT_CHECK([$abs_top_builddir/tests/context/context_test], [0], [expout], [ignore])
+AT_CLEANUP
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/13312
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Iae39cd57274bf6753ecaf186f229e582b42662e3
Gerrit-Change-Number: 13312
Gerrit-PatchSet: 19
Gerrit-Owner: laforge <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at gnumonks.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-CC: Max <suraev at alumni.ntnu.no>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190827/7385eef7/attachment.htm>