envoyproxy / envoyproxy/envoy

tls: find a generic solution for safely passing objects to worker threads

Open
#9,940 0 comments 0 reactions 0 assignees View on GitHub
help wanted tech debt
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
437

Description

When using TLS Slot::set with a callback, it's sometimes useful to add `this` the to the callback when creating the per thread state. For example, from a filter config constructor, one may do:
```
FilterConfig::FilterConfig(ThreadLocal::Instance& thread_local):slot_(thread_local.allocateSlot()) {
slot_->set([this,](
Event::Dispatcher& dispatcher) -> ThreadLocal::ThreadLocalObjectSharedPtr {
return this->makeThreadLocalState();
});
}
```
This is problematic, as the lifetime of the FilterConfig in the main thread is independent of the lifetime of the callback that runs on the worker threads. And in fact FilterConfig may be destructed by the time the callback is run on the worker threads.

A fix for one such instance, and a discussion on how to solve this was initially done here:
https://github.com/envoyproxy/envoy/pull/9884

This issue was created for tracking until a generic solution is achieved.
Few notes from the discussion in #9884 :
- use a weak pointer to move the variables used into the thread callback. not calling the callback if failing to lock the weak pointer.
- use linting to make sure that `this` is not passed to Slot::set
- figure out an easy\generic way to guarantee that the callback uses a valid object. see one such suggestion [here](https://github.com/envoyproxy/envoy/pull/9884#issuecomment-582533160)

cc @mattklein123

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.