Hi Steve,
Steven Dake wrote:
> I have a long running program which essentially opens a ssh session then
> periodically runs an exec operation over the session in a new channel.
> I have implemented it as a non-blocking implementation. I am using 1.4.0.
>
> The code seems to work well but I notice there is a bit of a leak found
> by valgrind:
> ==6656== at 0x4A074CD: malloc (vg_replace_malloc.c:236)^M
> ==6656== by 0x4EAC420: _libssh2_transport_read (transport.c:458)^M
> ==6656== by 0x4EA0694: _libssh2_packet_requirev (packet.c:1219)^M
> ==6656== by 0x4E97526: _libssh2_channel_open (channel.c:233)^M
> ==6656== by 0x4E97A8C: libssh2_channel_open_ex (channel.c:352)^M
> ==6656== by 0x405C9F: assembly_ssh_exec (trans_ssh.c:116)^M
> ==6656== by 0x4C3754A: job_dispatch (loop_job.c:39)^M
> ==6656== by 0x4C35FA6: qb_loop_run (loop.c:45)^M
> ==6656== by 0x4034DB: main (caped.c:163)^M
>
> If I run my application for 8 hours, the leak adds up to 300k of memory
> consumed. I had a look into the code for several hours and don't see an
> immediate way to fix the problem. I'm not sure even what the problem
> is, as the channel should free all packets on libssh2_channel_free.
The leaked memory is the session->payload buffer, which is actually
never freed by libssh2 at all.
Please see if the attached patch fixes the leak while still allowing
your program to keep running for a long time. I'm not completely sure
if the payload would need to be freed explicitly in _session_free()
or if there will always be a packet in the packets list refering to
it. Unsure. It would be helpful if you could test more. If my theory
is correct then memcheck should still report exactly one packet
leaked, regardless of running time. Then I will add freeing also to
_session_free().
> is there a bug in libssh2 here that is known?
Not known bug. I think most don't have long running sessions.
//Peter
diff --git a/src/transport.c b/src/transport.c
index 057dcf5..639984c 100644
--- a/src/transport.c
+++ b/src/transport.c
@@ -142,6 +142,7 @@ decrypt(LIBSSH2_SESSION * session, unsigned char *source,
if (session->remote.crypt->crypt(session, source,
&session->remote.crypt_abstract)) {
LIBSSH2_FREE(session, p->payload);
+ p->payload = NULL;
return LIBSSH2_ERROR_DECRYPT;
}
@@ -217,6 +218,7 @@ fullpacket(LIBSSH2_SESSION * session, int encrypted /* 1 or
0 */ )
session->fullpacket_payload_len,
&session->remote.comp_abstract);
LIBSSH2_FREE(session, p->payload);
+ p->payload = NULL;
if(rc)
return rc;
@@ -448,6 +450,13 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
return LIBSSH2_ERROR_OUT_OF_BOUNDARY;
}
+ /* If a packet was already allocated we must free it now.
+ * It would be nicer to keep track of allocated size, and
+ * only realloc when really neccessary.
+ */
+ if (p->payload)
+ LIBSSH2_FREE(session, p->payload);
+
/* Get a packet handle put data into. We get one to
hold all data, including padding and MAC. */
p->payload = LIBSSH2_ALLOC(session, total_num);
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
