Skip to content

Commit

Permalink
backport: Prevent SEGFAULT when disabling listener (#13515) (#13882)
Browse files Browse the repository at this point in the history
* Prevent SEGFAULT when disabling listener (#13515)

This prevents the stop_listening overload action from causing
segmentation faults that can occur if the action is enabled after the
listener has already shut down.

Signed-off-by: Alex Konradi <akonradi@google.com>
Signed-off-by: Christoph Pakulski <christoph@tetrate.io>
  • Loading branch information
cpakulski authored Nov 5, 2020
1 parent 8620920 commit 79984e4
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 2 deletions.
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Minor Behavior Changes
Bug Fixes
---------
*Changes expected to improve the state of the world and are unlikely to have negative effects*
* listener: fix crash when disabling or re-enabling listeners due to overload while processing LDS updates.

Removed Config or Runtime
-------------------------
Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Version history
:titlesonly:

current
v1.16.0
v1.15.2
v1.15.1
v1.15.0
Expand Down
12 changes: 12 additions & 0 deletions source/server/connection_handler_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,18 @@ void ConnectionHandlerImpl::ActiveTcpListener::onAcceptWorker(
}
}

void ConnectionHandlerImpl::ActiveTcpListener::pauseListening() {
if (listener_ != nullptr) {
listener_->disable();
}
}

void ConnectionHandlerImpl::ActiveTcpListener::resumeListening() {
if (listener_ != nullptr) {
listener_->enable();
}
}

void ConnectionHandlerImpl::ActiveTcpListener::newConnection(
Network::ConnectionSocketPtr&& socket, std::unique_ptr<StreamInfo::StreamInfo> stream_info) {
// Find matching filter chain.
Expand Down
4 changes: 2 additions & 2 deletions source/server/connection_handler_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ class ConnectionHandlerImpl : public Network::ConnectionHandler,

// ActiveListenerImplBase
Network::Listener* listener() override { return listener_.get(); }
void pauseListening() override { listener_->disable(); }
void resumeListening() override { listener_->enable(); }
void pauseListening() override;
void resumeListening() override;
void shutdownListener() override { listener_.reset(); }

// Network::BalancedConnectionHandler
Expand Down
16 changes: 16 additions & 0 deletions test/server/connection_handler_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,22 @@ TEST_F(ConnectionHandlerTest, AddDisabledListener) {
handler_->addListener(absl::nullopt, *test_listener);
}

TEST_F(ConnectionHandlerTest, DisableListenerAfterStop) {
InSequence s;

Network::TcpListenerCallbacks* listener_callbacks;
auto listener = new NiceMock<Network::MockListener>();
TestListener* test_listener =
addListener(1, false, false, "test_listener", listener, &listener_callbacks);
EXPECT_CALL(*socket_factory_, localAddress()).WillOnce(ReturnRef(local_address_));
handler_->addListener(absl::nullopt, *test_listener);

EXPECT_CALL(*listener, onDestroy());

handler_->stopListeners();
handler_->disableListeners();
}

TEST_F(ConnectionHandlerTest, DestroyCloseConnections) {
InSequence s;

Expand Down

0 comments on commit 79984e4

Please sign in to comment.