From 508138b7947291e4805f5b0e77e3a78e44b16e58 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov <vdavydov@tarantool.org> Date: Fri, 21 Oct 2022 13:38:51 +0300 Subject: [PATCH] fiber: initialize thread-local cord on demand We're planning to introduce a basic C API for user read views (EE-only). Like all other box C API functions, the new API functions will use the existing box error C API for reporting errors. The problem is that a read view created using C API should be usable from user threads (started with the pthread lib) while the box error C API doesn't work in user threads, because those threads don't have the cord pointer initialized (a diagnostic area is stored in a cord object). To address this issue, let's create a new cord object automatically on first use of cord() if it wasn't created explicitly. Automatically created object is destroyed at thread exit (to achieve that, we use the C++ RAII concept). Closes #7814 NO_DOC=The C API documentation doesn't say anything about threads. Let's keep it this way for now. We're planning to introduce a new C API to work with threads in C modules. We'll update the doc when it's ready. --- .../unreleased/gh-7814-box-error-in-thread.md | 4 ++ src/lib/core/CMakeLists.txt | 1 + src/lib/core/cord_on_demand.cc | 53 +++++++++++++++++++ src/lib/core/cord_on_demand.h | 25 +++++++++ src/lib/core/fiber.c | 2 +- src/lib/core/fiber.h | 15 +++++- test/unit/error.c | 39 +++++++++++++- 7 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/gh-7814-box-error-in-thread.md create mode 100644 src/lib/core/cord_on_demand.cc create mode 100644 src/lib/core/cord_on_demand.h diff --git a/changelogs/unreleased/gh-7814-box-error-in-thread.md b/changelogs/unreleased/gh-7814-box-error-in-thread.md new file mode 100644 index 0000000000..eb1026fa32 --- /dev/null +++ b/changelogs/unreleased/gh-7814-box-error-in-thread.md @@ -0,0 +1,4 @@ +## feature/core + +* The box error C API (`box_error_set()`, `box_error_last()`, etc) can now be + used in threads started by user modules with the pthread library (gh-7814). diff --git a/src/lib/core/CMakeLists.txt b/src/lib/core/CMakeLists.txt index 8c0ee829a2..9aff018f74 100644 --- a/src/lib/core/CMakeLists.txt +++ b/src/lib/core/CMakeLists.txt @@ -43,6 +43,7 @@ set(core_sources tt_sigaction.c tt_strerror.c mp_util.c + cord_on_demand.cc ) if (ENABLE_BACKTRACE) diff --git a/src/lib/core/cord_on_demand.cc b/src/lib/core/cord_on_demand.cc new file mode 100644 index 0000000000..a47b3b313e --- /dev/null +++ b/src/lib/core/cord_on_demand.cc @@ -0,0 +1,53 @@ +/* + * SPDX-License-Identifier: BSD-2-Clause + * + * Copyright 2010-2022, Tarantool AUTHORS, please see AUTHORS file. + */ + +#include "cord_on_demand.h" + +#include <stdlib.h> + +#include "fiber.h" +#include "trivia/util.h" + +/** + * RAII wrapper around a cord object. + * + * A thread-local cord object is created on the first call to the get() method + * and destroyed when the thread exits. + */ +class CordOnDemand final { +public: + static cord *get() noexcept + { + thread_local CordOnDemand singleton; + return singleton.cord_ptr; + } + +private: + struct cord *cord_ptr; + + CordOnDemand() noexcept + { + cord_ptr = static_cast<struct cord *>( + xcalloc(1, sizeof(*cord_ptr))); + cord_create(cord_ptr, "unknown"); + } + + ~CordOnDemand() + { + cord_exit(cord_ptr); + cord_destroy(cord_ptr); + free(cord_ptr); + } + + CordOnDemand(CordOnDemand &other) = delete; + CordOnDemand &operator=(CordOnDemand &other) = delete; +}; + +struct cord * +cord_on_demand(void) +{ + return CordOnDemand::get(); +} diff --git a/src/lib/core/cord_on_demand.h b/src/lib/core/cord_on_demand.h new file mode 100644 index 0000000000..c0491c7341 --- /dev/null +++ b/src/lib/core/cord_on_demand.h @@ -0,0 +1,25 @@ +/* + * SPDX-License-Identifier: BSD-2-Clause + * + * Copyright 2010-2022, Tarantool AUTHORS, please see AUTHORS file. + */ + +#pragma once + +#ifdef __cplusplus +extern "C" { +#endif /* __cplusplus */ + +struct cord; + +/** + * On the first call, creates and returns a new thread-local cord object. + * On all subsequent calls in the same thread, returns the object created + * earlier. The cord object is destroyed at thread exit. + */ +struct cord * +cord_on_demand(void); + +#ifdef __cplusplus +} /* extern "C" */ +#endif /* __cplusplus */ diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c index d8ccac4f9f..aceb2c7520 100644 --- a/src/lib/core/fiber.c +++ b/src/lib/core/fiber.c @@ -1481,7 +1481,7 @@ box_region_truncate(size_t size) void cord_create(struct cord *cord, const char *name) { - cord() = cord; + cord_ptr = cord; slab_cache_set_thread(&cord()->slabc); cord->id = pthread_self(); diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h index 282c732448..694cebb602 100644 --- a/src/lib/core/fiber.h +++ b/src/lib/core/fiber.h @@ -36,6 +36,7 @@ #include <stdint.h> #include "tt_pthread.h" #include <tarantool_ev.h> +#include "cord_on_demand.h" #include "diag.h" #include "trivia/util.h" #include "small/mempool.h" @@ -783,7 +784,19 @@ struct cord { extern __thread struct cord *cord_ptr; -#define cord() cord_ptr +/** + * Returns a thread-local cord object. + * + * If the cord object wasn't initialized at thread start (cord_create() + * wasn't called), a cord object is created automatically and destroyed + * at thread exit. + */ +#define cord() ({ \ + if (unlikely(cord_ptr == NULL)) \ + cord_ptr = cord_on_demand(); \ + cord_ptr; \ +}) + #define fiber() cord()->fiber #define loop() (cord()->loop) diff --git a/test/unit/error.c b/test/unit/error.c index 9bed7df07d..b1f5e908e0 100644 --- a/test/unit/error.c +++ b/test/unit/error.c @@ -18,6 +18,7 @@ #include "unit.h" #include <float.h> +#include <pthread.h> static void test_payload_field_str(void) @@ -481,11 +482,46 @@ test_error_code(void) footer(); } +static void * +test_pthread_f(void *arg) +{ + (void)arg; + is(box_error_last(), NULL, "last error before set"); + box_error_raise(ER_ILLEGAL_PARAMS, "Test %d", 42); + box_error_t *err = box_error_last(); + isnt(err, NULL, "last error after set"); + is(strcmp(box_error_type(err), "ClientError"), 0, "last error type"); + is(box_error_code(err), ER_ILLEGAL_PARAMS, "last error code"); + is(strcmp(box_error_message(err), "Test 42"), 0, "last error message"); + box_error_clear(); + is(box_error_last(), NULL, "last error after clear"); + return NULL; +} + +static void +test_pthread(void) +{ + header(); + plan(6); + + pthread_attr_t attr; + fail_unless(pthread_attr_init(&attr) == 0); + fail_unless(pthread_attr_setdetachstate( + &attr, PTHREAD_CREATE_JOINABLE) == 0); + + pthread_t thread; + fail_unless(pthread_create(&thread, &attr, test_pthread_f, NULL) == 0); + fail_unless(pthread_join(thread, NULL) == 0); + + check_plan(); + footer(); +} + int main(void) { header(); - plan(10); + plan(11); random_init(); memory_init(); @@ -501,6 +537,7 @@ main(void) test_payload_clear(); test_payload_move(); test_error_code(); + test_pthread(); fiber_free(); memory_free(); -- GitLab