Avoid undefined behavior in pthread TLS (#26999)

See code commentary for an explanation.

Add an additional constructor to allow `log_linux.cc` to compile with
GPR_PTHREAD_TLS. Without it:

```
../../third_party/grpc/src/core/lib/gpr/log_linux.cc:78:33: error: no viable conversion from 'int' to 'grpc_core::PthreadTlsImpl<long>'
  static GPR_THREAD_LOCAL(long) tid = 0;
                                ^     ~
../../third_party/grpc/src/core/lib/gpr/tls.h:64:3: note: candidate constructor not viable: no known conversion from 'int' to 'const grpc_core::PthreadTlsImpl<long> &' for 1st argument
  PthreadTlsImpl(const PthreadTlsImpl&) = delete;
  ^
1 error generated.
```
pull/26914/head
Tamir Duberstein 4 years ago committed by GitHub
parent 5f00f9c8ca
commit 34338e5798
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 76
      src/core/lib/gpr/tls.h
  2. 4
      src/core/lib/iomgr/ev_epoll1_linux.cc
  3. 4
      src/core/lib/iomgr/ev_epollex_linux.cc
  4. 11
      src/core/lib/iomgr/ev_poll_posix.cc
  5. 2
      src/core/lib/iomgr/exec_ctx.cc
  6. 6
      src/core/lib/iomgr/exec_ctx.h
  7. 2
      src/core/lib/iomgr/executor.cc
  8. 4
      src/core/lib/iomgr/timer_generic.cc
  9. 5
      src/core/lib/surface/completion_queue.cc
  10. 4
      test/core/gpr/tls_test.cc
  11. 3
      test/cpp/end2end/nonblocking_test.cc

@ -28,12 +28,6 @@
Usage is the same as C++ thread_local. Declaring a thread local:
static GPR_THREAD_LOCAL(uint32_t) foo;
Initializing a thread local (must be done at library initialization time):
gpr_tls_init(foo);
Destroying a thread local:
gpr_tls_destroy(foo);
ALL functions here may be implemented as macros. */
namespace grpc_core {
@ -67,13 +61,43 @@ namespace grpc_core {
template <typename T>
class PthreadTlsImpl : TlsTypeConstrainer<T> {
public:
PthreadTlsImpl() = default;
PthreadTlsImpl(const PthreadTlsImpl&) = delete;
PthreadTlsImpl& operator=(const PthreadTlsImpl&) = delete;
void Init() { GPR_ASSERT(0 == pthread_key_create(&key_, nullptr)); }
void Destroy() { GPR_ASSERT(0 == pthread_key_delete(key_)); }
// Achtung! This class emulates C++ `thread_local` using pthread keys. Each
// instance of this class is a stand in for a C++ `thread_local`. Think of
// each `thread_local` as a *global* pthread_key_t and a type tag. An
// important consequence of this is that the lifetime of a `pthread_key_t`
// is precisely the lifetime of an instance of this class. To understand why
// this is, consider the following scenario given a fictional implementation
// of this class which creates and destroys its `pthread_key_t` each time
// a given block of code runs (all actions take place on a single thread):
//
// - instance 1 (type tag = T*) is initialized, is assigned `pthread_key_t` 1
// - instance 2 (type tag = int) is initialized, is assigned `pthread_key_t` 2
// - instances 1 and 2 store and retrieve values; all is well
// - instances 1 and 2 are de-initialized; their keys are released to the pool
//
// - another run commences
// - instance 1 receives key 2
// - a value is read from instance 1, it observes a value of type int, but
// interprets it as T*; undefined behavior, kaboom
//
// To properly ensure these invariants are upheld the `pthread_key_t` must be
// `const`, which means it can only be released in the destructor. This is a
// a violation of the style guide, since these objects are always static (see
// footnote) but this code is used in sufficiently narrow circumstances to
// justify the deviation.
//
// https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables
PthreadTlsImpl()
: key_([]() {
pthread_key_t key;
GPR_ASSERT(0 == pthread_key_create(&key, nullptr));
return key;
}()) {}
PthreadTlsImpl(T t) : PthreadTlsImpl() { *this = t; }
~PthreadTlsImpl() { GPR_ASSERT(0 == pthread_key_delete(key_)); }
operator T() const { return Cast<T>(pthread_getspecific(key_)); }
@ -110,46 +134,18 @@ class PthreadTlsImpl : TlsTypeConstrainer<T> {
return reinterpret_cast<void*>(uintptr_t(t));
}
pthread_key_t key_;
};
// This class is never instantiated. It exists to statically ensure that all
// TLS usage is compatible with the most restrictive implementation, allowing
// developers to write correct code regardless of the platform they develop on.
template <typename T>
class TriviallyDestructibleAsserter {
// This type is often used as a global; since the type itself is hidden by the
// macros, enforce compliance with the style guide here rather than at the
// caller. See
// https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables.
static_assert(std::is_trivially_destructible<T>::value,
"TLS wrapper must be trivially destructible");
public:
using Type = T;
const pthread_key_t key_;
};
} // namespace grpc_core
#define GPR_THREAD_LOCAL(type) \
grpc_core::TriviallyDestructibleAsserter< \
grpc_core::PthreadTlsImpl<type>>::Type
#define gpr_tls_init(tls) (tls).Init()
#define gpr_tls_destroy(tls) (tls).Destroy()
#define GPR_THREAD_LOCAL(type) grpc_core::PthreadTlsImpl<type>
#else
#define GPR_THREAD_LOCAL(type) \
thread_local grpc_core::TlsTypeConstrainer<type>::Type
#define gpr_tls_init(tls) \
do { \
} while (0)
#define gpr_tls_destroy(tls) \
do { \
} while (0)
#endif
#endif /* GRPC_CORE_LIB_GPR_TLS_H */

@ -515,8 +515,6 @@ static size_t choose_neighborhood(void) {
}
static grpc_error_handle pollset_global_init(void) {
gpr_tls_init(g_current_thread_pollset);
gpr_tls_init(g_current_thread_worker);
gpr_atm_no_barrier_store(&g_active_poller, 0);
global_wakeup_fd.read_fd = -1;
grpc_error_handle err = grpc_wakeup_fd_init(&global_wakeup_fd);
@ -538,8 +536,6 @@ static grpc_error_handle pollset_global_init(void) {
}
static void pollset_global_shutdown(void) {
gpr_tls_destroy(g_current_thread_pollset);
gpr_tls_destroy(g_current_thread_worker);
if (global_wakeup_fd.read_fd != -1) grpc_wakeup_fd_destroy(&global_wakeup_fd);
for (size_t i = 0; i < g_num_neighborhoods; i++) {
gpr_mu_destroy(&g_neighborhoods[i].mu);

@ -648,15 +648,11 @@ static GPR_THREAD_LOCAL(grpc_pollset_worker*) g_current_thread_worker;
/* Global state management */
static grpc_error_handle pollset_global_init(void) {
gpr_tls_init(g_current_thread_pollset);
gpr_tls_init(g_current_thread_worker);
return pollable_create(PO_EMPTY, &g_empty_pollable);
}
static void pollset_global_shutdown(void) {
POLLABLE_UNREF(g_empty_pollable, "g_empty_pollable");
gpr_tls_destroy(g_current_thread_pollset);
gpr_tls_destroy(g_current_thread_worker);
}
/* pollset->mu must be held while calling this function */

@ -831,16 +831,9 @@ static grpc_error_handle pollset_kick(grpc_pollset* p,
/* global state management */
static grpc_error_handle pollset_global_init(void) {
gpr_tls_init(g_current_thread_poller);
gpr_tls_init(g_current_thread_worker);
return GRPC_ERROR_NONE;
}
static grpc_error_handle pollset_global_init(void) { return GRPC_ERROR_NONE; }
static void pollset_global_shutdown(void) {
gpr_tls_destroy(g_current_thread_poller);
gpr_tls_destroy(g_current_thread_worker);
}
static void pollset_global_shutdown(void) {}
/* main interface */

@ -138,7 +138,6 @@ ApplicationCallbackExecCtx::callback_exec_ctx_;
// WARNING: for testing purposes only!
void ExecCtx::TestOnlyGlobalInit(gpr_timespec new_val) {
g_start_time = new_val;
gpr_tls_init(exec_ctx_);
}
void ExecCtx::GlobalInit(void) {
@ -149,7 +148,6 @@ void ExecCtx::GlobalInit(void) {
g_start_time = gpr_now(GPR_CLOCK_MONOTONIC);
const gpr_cycle_counter cycle_after = gpr_get_cycle_counter();
g_start_cycle = (cycle_before + cycle_after) / 2;
gpr_tls_init(exec_ctx_);
}
bool ExecCtx::Flush() {

@ -216,7 +216,7 @@ class ExecCtx {
static void GlobalInit(void);
/** Global shutdown for ExecCtx. Called by iomgr. */
static void GlobalShutdown(void) { gpr_tls_destroy(exec_ctx_); }
static void GlobalShutdown(void) {}
/** Gets pointer to current exec_ctx. */
static ExecCtx* Get() { return exec_ctx_; }
@ -357,10 +357,10 @@ class ApplicationCallbackExecCtx {
}
/** Global initialization for ApplicationCallbackExecCtx. Called by init. */
static void GlobalInit(void) { gpr_tls_init(callback_exec_ctx_); }
static void GlobalInit(void) {}
/** Global shutdown for ApplicationCallbackExecCtx. Called by init. */
static void GlobalShutdown(void) { gpr_tls_destroy(callback_exec_ctx_); }
static void GlobalShutdown(void) {}
static bool Available() { return Get() != nullptr; }

@ -464,6 +464,6 @@ void Executor::SetThreadingDefault(bool enable) {
executors[static_cast<size_t>(ExecutorType::DEFAULT)]->SetThreading(enable);
}
void grpc_executor_global_init() { gpr_tls_init(g_this_thread_state); }
void grpc_executor_global_init() {}
} // namespace grpc_core

@ -270,7 +270,7 @@ static void timer_list_init() {
g_shared_mutables.min_timer = grpc_core::ExecCtx::Get()->Now();
#if GPR_ARCH_64
gpr_tls_init(g_last_seen_min_timer);
g_last_seen_min_timer = 0;
#endif
@ -303,7 +303,7 @@ static void timer_list_shutdown() {
gpr_mu_destroy(&g_shared_mutables.mu);
#if GPR_ARCH_64
gpr_tls_destroy(g_last_seen_min_timer);
#endif
gpr_free(g_shards);

@ -439,10 +439,7 @@ grpc_core::TraceFlag grpc_cq_pluck_trace(false, "queue_pluck");
static void on_pollset_shutdown_done(void* arg, grpc_error_handle error);
void grpc_cq_global_init() {
gpr_tls_init(g_cached_event);
gpr_tls_init(g_cached_cq);
}
void grpc_cq_global_init() {}
void grpc_completion_queue_thread_local_cache_init(grpc_completion_queue* cq) {
if (g_cached_cq == nullptr) {

@ -52,8 +52,6 @@ int main(int argc, char* argv[]) {
grpc::testing::TestEnvironment env(argc, argv);
gpr_tls_init(test_var);
for (auto& th : threads) {
th = grpc_core::Thread("grpc_tls_test", thd_body, nullptr);
th.Start();
@ -62,7 +60,5 @@ int main(int argc, char* argv[]) {
th.Join();
}
gpr_tls_destroy(test_var);
return 0;
}

@ -201,7 +201,6 @@ int main(int argc, char** argv) {
grpc::testing::TestEnvironment env(argc, argv);
::testing::InitGoogleTest(&argc, argv);
gpr_tls_init(g_is_nonblocking_poll);
// Start the nonblocking poll thread-local variable as false because the
// thread that issues RPCs starts by picking a port (which has non-zero
@ -209,7 +208,7 @@ int main(int argc, char** argv) {
g_is_nonblocking_poll = false;
int ret = RUN_ALL_TESTS();
gpr_tls_destroy(g_is_nonblocking_poll);
return ret;
#else // GRPC_POSIX_SOCKET
(void)argc;

Loading…
Cancel
Save