Fixed critical segfault, memory leak and default keepalive #54
Conversation
argakon
changed the title
| @@ -1147,7 +1147,12 @@ PHP_MOSQUITTO_API void php_mosquitto_message_callback(struct mosquitto *mosq, vo | |||
|
|
|||
| object_init_ex(message_zval, mosquitto_ce_message); | |||
| message_object = mosquitto_message_object_from_zend_object(Z_OBJ_P(message_zval)); | |||
| mosquitto_message_copy(&message_object->message, message); | |||
| message_object->message.mid = message->mid; | |||
There was a problem hiding this comment.
This caused a leak? Was it logged by PHP's memory leak detection or was it something you found in Valgrind or similar?
There was a problem hiding this comment.
Yes.
At first i used 2 simple scripts. One for subscribing and second for publishing 100000 messages. I run this scripts and after that watch in htop linux utility at memory column. In htop memory always grow.
After that i run subscribe script in valgrind and saw the problem. Leak was in PHP5 and PHP7.
setCredentials("esp", "esp"); $mqtt->onConnect(function($rc, $message) use ($topic, $mqtt) { echo 'Connected' . PHP_EOL; $mqtt->subscribe($topic, 0); }); $mqtt->onMessage(function($message) { var_dump(memory_get_usage()); var_dump($message); }); $mqtt->connect('192.168.5.1', 1883, 60); $mqtt->loopForever(100); ?> setCredentials("esp", "esp"); $iterations = 100000; $i = 0; echo 'Start testing' . PHP_EOL; $mqtt->connect("192.168.5.1", 1883, 60); while(true) { $arr = [ "test_i" => 123, "test_f" => 123.23, "test_s" => "test" . rand(1, 200000) ]; $s = $mqtt->publish("meteo/test", json_encode($arr)); usleep(1000); $i++; echo "Iteration: {$i}" . PHP_EOL; if ($i >= $iterations) break; } $mqtt->disconnect(); ?>
There was a problem hiding this comment.
Ahh, yes, I see now that I've looked at how that function is implemented:
https://github.com/eclipse/mosquitto/blob/master/lib/messages_mosq.c#L59
Good catch, thanks!
| message_object->message.qos = message->qos; | ||
| message_object->message.retain = message->retain; | ||
| message_object->message.topic = message->topic; | ||
| message_object->message.payload = message->payload; |
There was a problem hiding this comment.
I think we're going to have to estrdup() the topic and something similar for the payload, as Mosquitto will free the memory after the callback completes. I can do that though.
There was a problem hiding this comment.
As you wish. I'am not a C developer, my commit is a just quick fix.
No description provided.