BewareMyPower commented on code in PR #317:
URL: https://github.com/apache/pulsar-client-cpp/pull/317#discussion_r1330204269
##########
lib/ClientConnection.cc:
##########
@@ -597,10 +636,15 @@ void ClientConnection::handleResolve(const
boost::system::error_code& err,
void ClientConnection::readNextCommand() {
const static uint32_t minReadSize = sizeof(uint32_t);
+ auto weakSelf = weak_from_this();
asyncReceive(
incomingBuffer_.asio_buffer(),
- customAllocReadHandler(std::bind(&ClientConnection::handleRead,
shared_from_this(),
Review Comment:
I think not. If `ClientConnection` is destroyed when calling
`shared_from_this()`, `std::bad_weak_ptr` will be thrown. See the following
example:
```c++
#include <memory>
#include <iostream>
struct Demo : std::enable_shared_from_this<Demo> {
void f() {
auto self = shared_from_this();
std::cout << self->x << std::endl;
}
int x = 100;
};
int main() {
auto demo = std::make_shared<Demo>();
auto p = demo.get();
demo.reset();
p->f();
}
```
I built it and ran on CentOS 7 with GCC 7 and saw:
```
terminate called after throwing an instance of 'std::bad_weak_ptr'
what(): bad_weak_ptr
Aborted
```
In addition, `readNextCommand` is only called in `handleSentPulsarConnect`,
which is also called in the `std::bind` wrapper with `shared_from_this()`:
```c++
asyncWrite(buffer.const_asio_buffer(),
std::bind(&ClientConnection::handleSentPulsarConnect,
shared_from_this(),
std::placeholders::_1, buffer));
```
So when `handleSentPulsarConnect` is called, the
`shared_ptr<ClientConnection>` should not be expired.
----
The analysis above is based on the assumption that there is nothing wrong
with the following use:
```c++
// When f() is called, since shared_from_this() is passed to std::bind, the
ClientConnection object should be valid
async_xxx(xxx, std::bind(&ClientConnection::f, shared_from_this(), args...));
```
So I suspect that there might be something wrong with how we use
`std::bind`. Replacing it with lambda is more efficient, readable and **the
null check is performed explicitly**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
