How to handle client out of scope?
marcel-behlau-elfin opened this issue · 4 comments
In my application i observed problems, when a mqtt_cpp client is going out of scope. I created a minimal example for this:
https://github.com/marcel-behlau-elfin/mqtt_cpp/blob/client_out_of_scope/example/no_tls_client.cpp (diff)
I started a mosquitto in parallel by mosquitto -c example/mosquitto.conf
If i execute the modified no_tls_client
the application crashes with a core dump.
[marcel@elfin-behlau build]$ ./example/no_tls_client localhost 1883
c.use_count before connect: 1
c.use_count after connect: 2
Bye client scope
Connack handler called
Session Present: false
Connack Return Code: accepted
suback received. packet_id: 1
[client] subscribe result: success_maximum_qos_0
suback received. packet_id: 2
[client] subscribe result: success_maximum_qos_1
[client] subscribe result: success_maximum_qos_2
publish received. dup: no qos: at_most_once retain: no
topic_name: mqtt_client_cpp/topic1
contents: test1
Segmentation fault (core dumped)
This occurs, since the assigned callbacks of the client are still called after the client is gone out off scope. I can fix this, by assign a nullptr to the callbacks, before leaving the scope.
I'm not aware of the source code of mqtt_cpp, but i expect, that the callback is called from an freed object, so that the reset didn't fix the behaviour at all.
What is the correct implementation to free a mqtt_client, before going out of scope? Does the mqtt_cpp client need to clean the asio callbacks while destruction?
I also observed that the use_count of the smart pointer increases after the connect call. I don't see any reason for this. Is this intended?
*) This problem is still a memory issue and depends on runtime/compiled binary. I added the timer, since it causes the problem more often (at least, what i noticed)
There are some ways. They depends on your application requirement.
Some how keep the client lifetime
You can store the client as a member function of some object that have longer lifetime.
For example
class long_living_manager {
std::vector<decltype(c)> vec;
};
Close before the client become out of scode
After close callback or error callback is called, no more callback would be called. So if you can keep the client lifetime until close/error callback, then scope out is safe.
This is good for a kind of "one-shot" mqtt operation.
Use weak_ptr to check the client lifetime
You can get weak_type as follows:
using weak_type = typename decltype(c)::weak_type;
And then capture and use it as follows:
c->set_connack_handler(
[&c, &pid_sub1, &pid_sub2, wp = weak_type(c)]
(bool sp, MQTT_NS::connect_return_code connack_return_code){
if (auto shared_ptr = wp.lock()) {
// do some
}
return true;
}
);
Thank you very much for your fast response.
It's take some time to understand your reply. I'm not as much within the mqtt_cpp source code, so let me know/correct me, if one of my assumptions is wrong.
- The
connect
call creates viastd::shared_from_this
a copy of the client shared_ptr. - This
shared_ptr
lives within the asio loop and it's used for all operations - My original problem was, that my shared_ptr copy was gone out of scope.
- The client is still alive, since the copy within the asio loop
Okay, so there isn't a memory problem.
Does mqtt_cpp provides a mechanism to liftetime check my original client object? My expectation is, if the client is gone out of scope, the mqtt connection is automatically disconnect and the memory is freed. This is not the case. The disconnect must be done manually via a disconnect call. If a plain mqtt_client object is gone out of scope (e.g. an exception occurs), a disconnect call is not possible. Using the weak_ptr
within the registered callbacks, isn't an option to detect the lifetime.
If you could answer this question, i will close the issue. Thank you very much in advance!
Does mqtt_cpp provides a mechanism to liftetime check my original client object?
No. There is no such mechanism like other network communication libraries. e.g.) Boost.Asio
Okay, thank you very much for your feedback!