Skip to content

Commit

Permalink
Fix lambda caputre issue when we call overload callbacks.
Browse files Browse the repository at this point in the history
Summary:
There was some crashes inside `dropIdleConnectionsBasedOnTimeout` T165248097

After observing those I realised that issue was only happening when both asyncrhonous dropIDleConnection and on demand dropidleconnection was happening.. We have two place where we drop idle connections:
1. Asynchronously using adaptive idle timeouts
2. Synchronously inside HTTPRevproxyAcceptor (we should probably deprecate it)

I reproduced issue by running both of those at the same time and I managed to crash it again, here are some logs I've got:
```
E1017 00:15:12.386286   513 ExceptionTracer.cpp:222] exception stack complete
terminate called recursively
terminate called recursively
E1017 00:15:12.386209   523 ExceptionTracer.cpp:222] exception stack complete
terminate called recursively
E1017 00:15:12.238054   502 ExceptionTracer.cpp:220] Exception type: std::bad_function_call (17 frames)
    @ 0000000004d9245c __cxa_throw
                       ./fbcode/folly/experimental/exception_tracer/ExceptionTracerLib.cpp:75
    @ 000000000009d5d5 std::__throw_bad_function_call()
    @ 0000000006a84bd8
```

most important thing here was bad function call, turns out it was not anything like race condition/etc. one of the callback was getting out of scope before it would have been used.

I've refactored code to attach callback to `HTTPRevproxyService` this way we know that until `HTTPRevproxyService` exists this callback will exist, our load shedding mechanism's lifetime is attached to HTTPRevproxyService

Here is repro of this issue: D50396127

Reviewed By: meleshuk

Differential Revision: D50347506

fbshipit-source-id: b35a1cdabfc93304950271e1362ec974aee6038c
  • Loading branch information
Giorgi Papakerashvili authored and facebook-github-bot committed Oct 24, 2023
1 parent 0ce9273 commit 125e327
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion wangle/acceptor/Acceptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ void Acceptor::dropEstablishedConnections(
void Acceptor::dropIdleConnectionsBasedOnTimeout(
std::chrono::milliseconds targetIdleTimeMs,
const std::function<void(size_t)>& droppedConnectionsCB) {
base_->runInEventBaseThread([&] {
base_->runInEventBaseThread([this, targetIdleTimeMs, droppedConnectionsCB] {
if (downstreamConnectionManager_) {
VLOG(3) << "Dropping connections based on idle timeout "
<< targetIdleTimeMs.count() << " from acceptor=" << this
Expand Down

0 comments on commit 125e327

Please sign in to comment.