Hey folks,
I've found a memleak in the conference bridge.
When you configure PJ to use Speex for resampling,
a memory leak occurs when a conference port is removed
that has resample instances associated with it.
Resamples instances are created during the call to create_conf_port()
but are not destroyed anywhere. This in combination with third party
implementations
leads to a memory leak because third party implementations (like Speex)
do allocate memory internally.
The memory allocated internally by any external library is not
tracked/managed by PJ of course,
so simply "forgetting" the resample instance doesn't work here.
I've found this by accident while I was looking after another problem in
my code.
Please see the attached patch for a fix of this issue.
I ran my app through Valgrind's memcheck and it didn't find any other
issues.
While I was looking at the conference bridge, I noticed something else,
please see the comments marked "AWH".
I think there might be another leak related to passive ports which
brings me to my next point:
The function pjmedia_conf_add_passive_port() is (according to the log
statement inside) deprecated since pjsip 1.3 (which feels like eons away).
How about #ifdef'ing the function out by default?
That way it would not appear in the public API by default and if
somebody really needed it,
they could still enable it with an #ifdef in config_site.h
This is merely meant to make people aware that this function is going away.
Log statements can be ignored but a missing function cannot and a
developer would have to become active to reenable this function.
Just an idea...
Best regards,
Andreas Wehrmann
Index: pjmedia/src/pjmedia/conference.c
===================================================================
--- pjmedia/src/pjmedia/conference.c (revision 6069)
+++ pjmedia/src/pjmedia/conference.c (working copy)
@@ -667,7 +667,18 @@
continue;
++ci;
+ if(cport->rx_resample) {
+ pjmedia_resample_destroy(cport->rx_resample);
+ cport->rx_resample = NULL;
+ }
+
+ if(cport->tx_resample) {
+ pjmedia_resample_destroy(cport->tx_resample);
+ cport->tx_resample = NULL;
+ }
+
if (cport->delay_buf) {
+ /* AWH: should we not destroy the port here, if it exists? */
pjmedia_delay_buf_destroy(cport->delay_buf);
cport->delay_buf = NULL;
}
@@ -1173,12 +1184,25 @@
--conf->connect_cnt;
}
+ /* Destroy resample if this conf port has it.
+ */
+ if(conf_port->rx_resample) {
+ pjmedia_resample_destroy(conf_port->rx_resample);
+ conf_port->rx_resample = NULL;
+ }
+
+ if(conf_port->tx_resample) {
+ pjmedia_resample_destroy(conf_port->tx_resample);
+ conf_port->tx_resample = NULL;
+ }
+
/* Destroy pjmedia port if this conf port is passive port,
* i.e: has delay buf.
*/
if (conf_port->delay_buf) {
pjmedia_port_destroy(conf_port->port);
conf_port->port = NULL;
+ /* AWH: should we not destroy the delay buffer here as well? */
}
/* Remove the port. */
_______________________________________________
Visit our blog: http://blog.pjsip.org
pjsip mailing list
pjsip@xxxxxxxxxxxxxxx
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
