Correct table clear bug in route table ready

This change fixes a bug introduced in the 4.4.0 fix. During
symtable roll prep the table was cleared, but the reference
counts in the RTEs was not being decremented.

Issue-ID: RIC-674

Signed-off-by: E. Scott Daniels <daniels@research.att.com>
Change-Id: I3b1fb0a74207960b2adeb9c53016358d01658b1b
Signed-off-by: E. Scott Daniels <daniels@research.att.com>

diff --git a/CHANGES_CORE.txt b/CHANGES_CORE.txt
index d448a57..5d54c19 100644 (file)
--- 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.3
+       Correct bug introduced with race fix (4.4.0) (RIC-674)
+
 2020 November 4; Version 4.4.2
        Changes to correct more complaints generated by a code scan. (RIC-673)
        Also addressed some sonar coverage issues with unit test changes.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8705898..407a5a9 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -41,7 +41,7 @@ cmake_minimum_required( VERSION 3.5 )
 
 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 "2" )
+set( patch_level "3" )
 
 set( install_root "${CMAKE_INSTALL_PREFIX}" )
 set( install_inc "include/rmr" )

diff --git a/docs/rel-notes.rst b/docs/rel-notes.rst
index 7b3d7a7..b7faccb 100644 (file)
--- a/docs/rel-notes.rst
+++ b/docs/rel-notes.rst
@@ -22,11 +22,27 @@ the need to leap frog versions ceased, and beginning with
 version 4.0.0, the RMR versions should no longer skip.
 
 
+2020 November 4; Version 4.4.3
+------------------------------
+
+Correct bug introduced with race fix (4.4.0) (RIC-674)
+
+
+
+2020 November 4; Version 4.4.2
+------------------------------
+
+Changes to correct more complaints generated by a code scan.
+(RIC-673) Also addressed some sonar coverage issues with unit
+test changes.
+
+
+
 2020 November 4; Version 4.4.1
 ------------------------------
 
 Changes to correct complaints generated by a code scan.
-(Ric-673)
+(RIC-673)
 
 
 

diff --git a/src/rmr/common/src/rt_generic_static.c b/src/rmr/common/src/rt_generic_static.c
index 84f8445..bf7bfdc 100644 (file)
--- a/src/rmr/common/src/rt_generic_static.c
+++ b/src/rmr/common/src/rt_generic_static.c
@@ -291,7 +291,7 @@ static void send_rt_ack( uta_ctx_t* ctx, rmr_mbuf_t* smsg, char* table_id, int s
 
                smsg->len = strlen( smsg->payload ) + 1;
 
-               rmr_vlog( RMR_VL_INFO, "rmr_rtc: sending table state: (%s) state=%d whid=%d\n", smsg->payload, state, ctx->rtg_whid );
+               rmr_vlog( RMR_VL_INFO, "rmr_rtc: sending table state: (%s) state=%d whid=%d table=%s\n", smsg->payload, state, ctx->rtg_whid, table_id );
                if( use_rts ) {
                        smsg = rmr_rts_msg( ctx, smsg );
                } else {
@@ -916,6 +916,7 @@ static void parse_rt_rec( uta_ctx_t* ctx,  uta_ctx_t* pctx, char* buf, int vleve
                                                if( ctx->new_rtable->updates != atoi( tokens[2] ) ) {   // count they added didn't match what we received
                                                        rmr_vlog( RMR_VL_ERR, "rmr_rtc: RT update had wrong number of records: received %d expected %s\n",
                                                                ctx->new_rtable->updates, tokens[2] );
+                                                       send_rt_ack( pctx, mbuf, ctx->table_id, !RMR_OK, wbuf );
                                                        uta_rt_drop( ctx->new_rtable );
                                                        ctx->new_rtable = NULL;
                                                        break;
@@ -936,6 +937,9 @@ static void parse_rt_rec( uta_ctx_t* ctx,  uta_ctx_t* pctx, char* buf, int vleve
                                                        rmr_vlog_force( RMR_VL_DEBUG, "updated route table:\n" );
                                                        rt_stats( ctx->rtable );
                                                }
+
+                                               send_rt_ack( pctx, mbuf, ctx->table_id, RMR_OK, NULL );
+                                               ctx->rtable_ready = 1;                                                  // route based sends can now happen
                                        } else {
                                                if( DEBUG > 1 ) rmr_vlog_force( RMR_VL_DEBUG, "end of rt update noticed, but one was not started!\n" );
                                                ctx->new_rtable = NULL;
@@ -943,6 +947,7 @@ static void parse_rt_rec( uta_ctx_t* ctx,  uta_ctx_t* pctx, char* buf, int vleve
                                } else {                                                                                        // start a new table.
                                        if( ctx->new_rtable != NULL ) {                                 // one in progress?  this forces it out
                                                if( DEBUG > 1 || (vlevel > 1) ) rmr_vlog_force( RMR_VL_DEBUG, "new table; dropping incomplete table\n" );
+                                               send_rt_ack( pctx, mbuf, ctx->table_id, !RMR_OK, "table not complete" );                        // nack the one that was pending as end never made it
                                                uta_rt_drop( ctx->new_rtable );
                                                ctx->new_rtable = NULL;
                                        }
@@ -1300,7 +1305,10 @@ static route_table_t* prep_new_rt( uta_ctx_t* ctx, int all ) {
                        usleep( 1000 );                                         // small sleep to yield the processer if that is needed
                }
 
-               rmr_sym_clear( rt );                                    // clear all entries from the old table
+               if( rt->hash != NULL ) {
+                       rmr_sym_foreach_class( rt->hash, 0, del_rte, NULL );            // deref and drop if needed
+                       rmr_sym_clear( rt->hash );                                                                      // clear all entries from the old table
+               }
        } else {
                rt = NULL;
        }

diff --git a/src/rmr/si/src/rtable_si_static.c b/src/rmr/si/src/rtable_si_static.c
index c348b7f..f7cddf5 100644 (file)
--- a/src/rmr/si/src/rtable_si_static.c
+++ b/src/rmr/si/src/rtable_si_static.c
@@ -224,12 +224,15 @@ static int uta_epsock_byname( uta_ctx_t* ctx, char* ep_name, int* nn_sock, endpo
 
        if( PARANOID_CHECKS ) {
                if( ctx == NULL ) {
-                       if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "epsock_byname: parinoia check pop ctx=%p rt=%p\n", ctx, rt );
+                       if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "epsock_byname: paranoia check pop ctx=%p rt=%p\n", ctx, rt );
                        return FALSE;
                }
-               rt = get_rt( ctx );                             // get active rt and bump ref count
-               if( rt == NULL || (si_ctx = ctx->si_ctx) == NULL  ) {
-                       if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "epsock_byname: parinoia check pop rt=%p sictx=%p\n", rt, si_ctx );
+               if( (si_ctx = ctx->si_ctx) == NULL ) {
+                       if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "epsock_byname: paranoia check pop sictx is nil\n" );
+                       return FALSE;
+               }
+               if( (rt = get_rt( ctx )) == NULL ) {                            // get active rt and bump ref count
+                       if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "epsock_byname: paranoia check pop no rtable\n" );
                        return FALSE;
                }
        } else {

diff --git a/test/rt_static_test.c b/test/rt_static_test.c
index 8bd91df..79d0561 100644 (file)
--- a/test/rt_static_test.c
+++ b/test/rt_static_test.c
@@ -127,6 +127,7 @@ static int rt_test( ) {
        char*   seed_fname;             // seed file
        SOCKET_TYPE     nn_sock;        // differnt in each transport (nng == struct, SI/Nano == int)
        rmr_mbuf_t*     mbuf;           // message for meid route testing
+       void*   p;                              // generic pointer
 
        #ifndef NNG_UNDER_TEST
                si_ctx_t* si_ctx = NULL;
@@ -294,6 +295,12 @@ static int rt_test( ) {
                state = uta_epsock_byname( NULL, "localhost:4561", &nn_sock, &ep );             // test coverage on nil checks
        #else
                state = uta_epsock_byname( NULL, "localhost:4561", &nn_sock, &ep );
+               errors += fail_not_equal( state, 0, "socket (by name) nil context check returned true" );
+
+               p = ctx->si_ctx;
+               ctx->si_ctx = NULL;             // set to drive second test
+               state = uta_epsock_byname( ctx, "localhost:4561", &nn_sock, &ep );
+               ctx->si_ctx = p;
        #endif
        errors += fail_not_equal( state, 0, "socket (by name) nil check returned true" );
 
@@ -436,9 +443,6 @@ static int rt_test( ) {
                                "mse | 1  | -1 | localhost:84306\n"
                                "mse | 10  | -1 | localhost:84306\n"
                                "mse | 10  | 1 | localhost:84306\n"
-                               "# short record to drive test\n"
-                               "del\n"
-                               "del | 12 | 12\n"
 
                                "# this table should be ok\n"
                                "newrt | start | dummy-seed\n"
@@ -448,21 +452,24 @@ static int rt_test( ) {
                                "newrt | end | 3\n"
 
                                "# for an update to the existing table\n"
-
                                "# not in progress; drive that exception check\n"
                                "update | end | 23\n"
 
                                "update | start | dummy-seed\n"
-                               "mse | 2 | 2 | localhost:2222\n"
+                               "mse | 3 | 2 | localhost:2222\n"
+                               "# short record to drive test\n"
+                               "del\n"
                                "# no table end for exception handling\n"
 
                                "update | start | dummy-seed\n"
                                "mse | 2 | 2 | localhost:2222\n"
-                               "update | end | 1\n";
+                               "del | 10 | 1\n"
+                               "update | end | 2\n";
 
                fprintf( stderr, "<INFO> loading RT from edge case static table\n" );
                fprintf( stderr, "<INFO> %s\n", rt_stuff );
                gen_custom_rt( ctx, rt_stuff );
+               fprintf( stderr, "<INFO> edge case load completed\n" );
                errors += fail_if_nil( ctx->rtable, "edge case route table didn't generate a pointer into the context" );
 
                unsetenv( "RMR_SEED_RT" );                      // remove for next read try