Skip to content

Commit cedc0c1

Browse files
Fix segfault caused by socket I/O on a closed io_service (#168)
Fixes #167 ### Motivation Here are some debugging info when the segfault happened in `testCloseClient`. The outputs have been trimmed to make them clear. An example crash at `async_write`: ``` #12 0x00007ffff7496dad in basic_stream_socket<...>::boost::asio::async_write /usr/include/boost/asio/impl/write.hpp:512 #13 0x00007ffff748e003 in ClientConnection::asyncWrite lib/ClientConnection.h:245 #14 0x00007ffff746e0b6 in ClientConnection::handleHandshake (this=0x555555e689d0) lib/ClientConnection.cc:502 ``` Another example crash at `async_receive`: ``` #6 0x00007ffff7497247 in basic_stream_socket<...>::async_receive /usr/include/boost/asio/basic_stream_socket.hpp:677 #7 0x00007ffff748e647 in ClientConnection::asyncReceive lib/ClientConnection.h:258 #8 0x00007ffff746fa5d in ClientConnection::readNextCommand lib/ClientConnection.cc:606 ``` The frame where it crashed: ``` 245 if (descriptor_data->shutdown_) (gdb) p descriptor_data $2 = (boost::asio::detail::epoll_reactor::per_descriptor_data &) @0x555555e4a780: 0x0 ``` We can see the socket descriptor is `nullptr`. The root cause is when `async_receive` or `async_write` is called, the `io_service` object might be closed. This case happened when `createProducerAsync` is called, the actual producer creation continues in another thread, while the `client.close()` happens in the current thread. ### Modifications Check if the `ClientConnection` is closed before `async_receive` or `async_write`. To avoid the use of lock, changing the `state_` field to atomic. ### Verifications ```bash ./tests/pulsar-tests --gtest_filter='ClientTest.testCloseClient' --gtest_repeat=20 ``` It never crashed after applying this patch.
1 parent a46da16 commit cedc0c1

File tree

2 files changed

+9
-2
lines changed

2 files changed

+9
-2
lines changed

lib/ClientConnection.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,7 @@ void ClientConnection::handleIncomingMessage(const proto::CommandMessage& msg, b
812812
void ClientConnection::handleIncomingCommand(BaseCommand& incomingCmd) {
813813
LOG_DEBUG(cnxString_ << "Handling incoming command: " << Commands::messageType(incomingCmd.type()));
814814

815-
switch (state_) {
815+
switch (state_.load()) {
816816
case Pending: {
817817
LOG_ERROR(cnxString_ << "Connection is not ready yet");
818818
break;

lib/ClientConnection.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <pulsar/ClientConfiguration.h>
2323
#include <pulsar/defines.h>
2424

25+
#include <atomic>
2526
#include <boost/any.hpp>
2627
#include <boost/asio/bind_executor.hpp>
2728
#include <boost/asio/deadline_timer.hpp>
@@ -235,6 +236,9 @@ class PULSAR_PUBLIC ClientConnection : public std::enable_shared_from_this<Clien
235236

236237
template <typename ConstBufferSequence, typename WriteHandler>
237238
inline void asyncWrite(const ConstBufferSequence& buffers, WriteHandler handler) {
239+
if (isClosed()) {
240+
return;
241+
}
238242
if (tlsSocket_) {
239243
#if BOOST_VERSION >= 106600
240244
boost::asio::async_write(*tlsSocket_, buffers, boost::asio::bind_executor(strand_, handler));
@@ -248,6 +252,9 @@ class PULSAR_PUBLIC ClientConnection : public std::enable_shared_from_this<Clien
248252

249253
template <typename MutableBufferSequence, typename ReadHandler>
250254
inline void asyncReceive(const MutableBufferSequence& buffers, ReadHandler handler) {
255+
if (isClosed()) {
256+
return;
257+
}
251258
if (tlsSocket_) {
252259
#if BOOST_VERSION >= 106600
253260
tlsSocket_->async_read_some(buffers, boost::asio::bind_executor(strand_, handler));
@@ -259,7 +266,7 @@ class PULSAR_PUBLIC ClientConnection : public std::enable_shared_from_this<Clien
259266
}
260267
}
261268

262-
State state_ = Pending;
269+
std::atomic<State> state_{Pending};
263270
TimeDuration operationsTimeout_;
264271
AuthenticationPtr authentication_;
265272
int serverProtocolVersion_;

0 commit comments

Comments
 (0)