Correct memory leak in the RTE cleanup
When cleaning up a route table entry the round robin group
block (rrg) was not being correctly freed. The leak was small,
but would be noticed in a situation described by the indicated
issue (many table updates).
Issue-ID: RIC-674
Signed-off-by: E. Scott Daniels <daniels@research.att.com>
Change-Id: Ica7d0219574abd33392c7127f918ac71b2891702
Signed-off-by: E. Scott Daniels <daniels@research.att.com>
diff --git a/CHANGES_CORE.txt b/CHANGES_CORE.txt
index 5d54c19..db2f2e6 100644
--- a/CHANGES_CORE.txt
+++ b/CHANGES_CORE.txt
@@ -5,6 +5,9 @@
# API and build change and fix summaries. Doc correctsions
# and/or changes are not mentioned here; see the commit messages.
+2020 November 4; Version 4.4.4
+ Correct address memory leak in the RTE cleanup. (RIC-674)
+
2020 November 4; Version 4.4.3
Correct bug introduced with race fix (4.4.0) (RIC-674)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 407a5a9..763c73f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -41,7 +41,7 @@
set( major_version "4" ) # should be automatically populated from git tag later, but until CI process sets a tag we use this
set( minor_version "4" )
-set( patch_level "3" )
+set( patch_level "4" )
set( install_root "${CMAKE_INSTALL_PREFIX}" )
set( install_inc "include/rmr" )
diff --git a/docs/rel-notes.rst b/docs/rel-notes.rst
index b7faccb..4384bdc 100644
--- a/docs/rel-notes.rst
+++ b/docs/rel-notes.rst
@@ -22,6 +22,13 @@
version 4.0.0, the RMR versions should no longer skip.
+2020 November 4; Version 4.4.4
+------------------------------
+
+Correct address memory leak in the RTE cleanup. (RIC-674)
+
+
+
2020 November 4; Version 4.4.3
------------------------------
diff --git a/src/rmr/common/src/rt_generic_static.c b/src/rmr/common/src/rt_generic_static.c
index bf7bfdc..116b17f 100644
--- a/src/rmr/common/src/rt_generic_static.c
+++ b/src/rmr/common/src/rt_generic_static.c
@@ -211,6 +211,18 @@
rmr_sym_foreach_class( rt->hash, 1, ep_counts, id ); // run endpoints in the active table
}
+
+static void dump_tables( uta_ctx_t *ctx ) {
+ if( ctx->old_rtable != NULL ) {
+ rmr_vlog_force( RMR_VL_DEBUG, "old route table: (ref_count=%d)\n", ctx->old_rtable->ref_count );
+ rt_stats( ctx->old_rtable );
+ } else {
+ rmr_vlog_force( RMR_VL_DEBUG, "old route table was empty\n" );
+ }
+ rmr_vlog_force( RMR_VL_DEBUG, "new route table:\n" );
+ rt_stats( ctx->rtable );
+}
+
// ------------ route manager communication -------------------------------------------------
/*
Send a request for a table update to the route manager. Updates come in
@@ -829,18 +841,10 @@
}
if( ctx->new_rtable ) {
- if( DEBUG > 1 || (vlevel > 1) ) rmr_vlog( RMR_VL_DEBUG, "end of route table noticed\n" );
roll_tables( ctx ); // roll active to old, and new to active with proper locking
-
- if( vlevel > 0 ) {
- if( ctx->old_rtable != NULL ) {
- rmr_vlog_force( RMR_VL_DEBUG, "old route table: (ref_count=%d)\n", ctx->old_rtable->ref_count );
- rt_stats( ctx->old_rtable );
- } else {
- rmr_vlog_force( RMR_VL_DEBUG, "old route table was empty\n" );
- }
- rmr_vlog_force( RMR_VL_DEBUG, "new route table:\n" );
- rt_stats( ctx->rtable );
+ if( DEBUG > 1 || (vlevel > 1) ) {
+ rmr_vlog( RMR_VL_DEBUG, "end of route table noticed\n" );
+ dump_tables( ctx );
}
send_rt_ack( pctx, mbuf, ctx->table_id, RMR_OK, NULL );
@@ -925,17 +929,9 @@
if( ctx->new_rtable ) {
roll_tables( ctx ); // roll active to old, and new to active with proper locking
- if( DEBUG > 1 || (vlevel > 1) ) rmr_vlog_force( RMR_VL_DEBUG, "end of rt update noticed\n" );
-
- if( vlevel > 0 ) {
- if( ctx->old_rtable != NULL ) {
- rmr_vlog_force( RMR_VL_DEBUG, "old route table: (ref_count=%d)\n", ctx->old_rtable->ref_count );
- rt_stats( ctx->old_rtable );
- } else {
- rmr_vlog_force( RMR_VL_DEBUG, "old route table was empty\n" );
- }
- rmr_vlog_force( RMR_VL_DEBUG, "updated route table:\n" );
- rt_stats( ctx->rtable );
+ if( DEBUG > 1 || (vlevel > 1) ) {
+ rmr_vlog_force( RMR_VL_DEBUG, "end of rt update noticed\n" );
+ dump_tables( ctx );
}
send_rt_ack( pctx, mbuf, ctx->table_id, RMR_OK, NULL );
@@ -1082,7 +1078,9 @@
for( i = 0; i < rte->nrrgroups; i++ ) {
if( rte->rrgroups[i] ) {
free( rte->rrgroups[i]->epts ); // ditch list of endpoint pointers (end points are reused; don't trash them)
+ free( rte->rrgroups[i] ); // but must free the rrg itself too
}
+
}
free( rte->rrgroups );
diff --git a/src/rmr/common/src/rtc_static.c b/src/rmr/common/src/rtc_static.c
index 15fe567..ee84c91 100644
--- a/src/rmr/common/src/rtc_static.c
+++ b/src/rmr/common/src/rtc_static.c
@@ -274,11 +274,6 @@
return NULL;
}
- if( (ctx->ephash = rmr_sym_alloc( RT_SIZE )) == NULL ) { // master hash table for endpoints (each rt will reference this)
- rmr_vlog( RMR_VL_CRIT, "rmr_rtc: internal mishap: unable to allocate an endpoint hash table\n" );
- return NULL;
- }
-
if( (eptr = getenv( ENV_VERBOSE_FILE )) != NULL ) {
vfd = open( eptr, O_RDONLY );
vlevel = refresh_vlevel( vfd );
diff --git a/src/rmr/common/src/tools_static.c b/src/rmr/common/src/tools_static.c
index cc95fc1..bbe49ee 100644
--- a/src/rmr/common/src/tools_static.c
+++ b/src/rmr/common/src/tools_static.c
@@ -187,7 +187,7 @@
*(tok++) = 0;
}
- hent = gethostbyname( dname );
+ hent = gethostbyname( dname ); // valgrind will complain that this leaks, but we cannot free it!
if( hent == NULL || hent->h_addr_list == NULL ) {
//rmr_vlog( RMR_VL_WARN, "h2ip: dns lookup failed for: %s\n", dname );
free( dname );
diff --git a/src/rmr/si/src/si95/siestablish.c b/src/rmr/si/src/si95/siestablish.c
index a62490d..794ea07 100644
--- a/src/rmr/si/src/si95/siestablish.c
+++ b/src/rmr/si/src/si95/siestablish.c
@@ -65,30 +65,24 @@
Returns a transport struct which is the main context for the listener.
*/
extern struct tp_blk *SIlisten_prep( int type, char* abuf, int family ) {
- struct tp_blk *tptr; // pointer at new tp block
- int status = SI_OK; // processing status
+ struct tp_blk *tptr; // pointer at new tp block
struct sockaddr *addr; // IP address we are requesting
- int protocol; // protocol for socket call
int optval = 0;
int alen = 0;
+ int status = SI_OK; // processing status
+ int protocol; // protocol for socket call
- tptr = (struct tp_blk *) SInew( TP_BLK ); // new transport info block
+ tptr = (struct tp_blk *) SInew( TP_BLK ); // transport info
- if( tptr != NULL )
- {
+ if( tptr != NULL ) {
addr = NULL;
- switch( type ) // things specifc to tcp or udp
- {
- case UDP_DEVICE:
- tptr->type = SOCK_DGRAM;
- protocol = IPPROTO_UDP;
- break;
-
- case TCP_DEVICE:
- default:
- tptr->type = SOCK_STREAM;
- protocol = IPPROTO_TCP;
+ if( type == UDP_DEVICE ) {
+ tptr->type = SOCK_DGRAM;
+ protocol = IPPROTO_UDP;
+ } else {
+ tptr->type = SOCK_STREAM;
+ protocol = IPPROTO_TCP;
}
alen = SIgenaddr( abuf, protocol, family, tptr->type, &addr ); // family == 0 for type that suits the address passed in
diff --git a/src/rmr/si/src/si95/sinewses.c b/src/rmr/si/src/si95/sinewses.c
index 02a5e6d..56d9b10 100644
--- a/src/rmr/si/src/si95/sinewses.c
+++ b/src/rmr/si/src/si95/sinewses.c
@@ -88,8 +88,8 @@
}
SETSOCKOPT( tpptr->fd, SOL_TCP, TCP_QUICKACK, (void *)&optval, sizeof( optval) ) ;
- SIaddress( addr, (void **) &buf, AC_TODOT ); // get addr of remote side
- if( (cbptr = gptr->cbtab[SI_CB_SECURITY].cbrtn) != NULL ) { // invoke the security callback function if there
+ SIaddress( addr, (void **) &buf, AC_TODOT ); // get addr of remote side; buf must be freed
+ if( (cbptr = gptr->cbtab[SI_CB_SECURITY].cbrtn) != NULL ) { // invoke the security callback function if there
status = (*cbptr)( gptr->cbtab[SI_CB_SECURITY].cbdata, buf );
if( status == SI_RET_ERROR ) { // session to be rejected
SIterm( gptr, newtp ); // terminate new tp block (do NOT call trash)
@@ -110,6 +110,7 @@
SImap_fd( gptr, newtp->fd, newtp ); // add fd to the map
+ free( buf );
return SI_OK;
}