mosquitto_reconnect seems memory leak when TLS error

[smartdabao]

I found it seems like memory leak when mosquitto_reconnect was used if TLS error. Plz reffer net_mosq.c line 880-901, version 1.4.14

while(packet->to_process > 0){
write_length = _mosquitto_net_write(mosq, &(packet->payload[packet->pos]), packet->to_process);
if(write_length > 0){
#if defined(WITH_BROKER) && defined(WITH_SYS_TREE)
g_bytes_sent += write_length;
#endif
packet->to_process -= write_length;
packet->pos += write_length;
}else{
#ifdef WIN32
errno = WSAGetLastError();
#endif
if(errno == EAGAIN || errno == COMPAT_EWOULDBLOCK){
pthread_mutex_unlock(&mosq->current_out_packet_mutex);
return MOSQ_ERR_SUCCESS; //The packet is not free here
}else{
pthread_mutex_unlock(&mosq->current_out_packet_mutex);
switch(errno){
case COMPAT_ECONNRESET:
return MOSQ_ERR_CONN_LOST; //The packet is not free here
default:
return MOSQ_ERR_ERRNO; //The packet is not free here
}
}
}
}

Is that right?

[toast-uz]

Could you show any evidence of the memory leak except for reviewing code?

[smartdabao]

We use TLS and mosquitto. The procedure is as follows.

1. mosquitto_connect is ok.
2. for NTP reason, TLS failed, lead to mosquitto_loop return ERROR. So we reconnect, and mosquitto_reconnect return OK. Every time call mosquitto_reconnect, memory will raise, until memory exhausted. Our code is as follows.
   while(run){
   rc = mosquitto_loop(mosq, -1, 1);
   if(run && rc){
   printf("connection error!\n");
   sleep(10);
   mosquitto_reconnect(mosq);
   }

[toast-uz]

I'm not familiar with libmosquitto, so I cannot reproduce your situation.
But I guess the code you pointed is not wrong.
Because the packet = mosq->current_out_packet is a part of mosq that is a parameter of function _mosquitto_packet_write, then it should be handled at the caller of the function.

[ralight]

I've not been able to reproduce your problem. I've used the partial bit of code you provided. Can you provide any more details?

[aaronovz1]

This is definitely still a problem in 1.5.3

[DEBUG] : Client 1234 sending CONNECT
[ERROR] : OpenSSL Error: error:14094416:SSL routines:ssl3_read_bytes:sslv3 alert certificate unknown
[ERROR] : loop_forever failed (8): A TLS error occurred.

==4572== 1,876,996 (21,424 direct, 1,855,572 indirect) bytes in 26 blocks are definitely lost in loss record 267 of 267
==4572==    at 0x4C2CECB: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4572==    by 0x714107: CRYPTO_malloc (in ...)
==4572==    by 0x6E7DBF: SSL_new (in ...)
==4572==    by 0x6D2C2A: net__socket_connect_step3 (net_mosq.c:599)
==4572==    by 0x6D3632: net__socket_connect (net_mosq.c:648)
==4572==    by 0x6D0BF7: mosquitto__reconnect.part.1 (connect.c:184)
==4572==    by 0x6D1DB1: mosquitto_loop_forever (loop.c:284)
==4572==    by 0x506D33: sm::comms::MQTTClient::eventLoop() (mqttclient.cpp:123)
==4572==    by 0x61F2A8: boost::function0<void>::operator()() const (function_template.hpp:769)
==4572==    by 0x431FCB: sm::thread::Thread::main() (thread.cpp:274)
==4572==    by 0x85E424: thread_proxy (thread.cpp:175)
==4572==    by 0x598C476: start_thread (pthread_create.c:463)

There is a check in connect.c that does the following:

if(mosq->sock != INVALID_SOCKET){
	net__socket_close(mosq); //close socket
}

If I remove the check and always close the socket, then the leaks go away.

[ralight]

Thanks for the hint, I believe this is now fixed. Are you able to check the fixes branch and report back?