From 353bafbe34c601eda6649ea7dcfdcf285d796d5a Mon Sep 17 00:00:00 2001 From: "E. Scott Daniels" Date: Thu, 12 Nov 2020 16:44:34 -0500 Subject: [PATCH] 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 Change-Id: I3b1fb0a74207960b2adeb9c53016358d01658b1b Signed-off-by: E. Scott Daniels --- CHANGES_CORE.txt | 3 +++ CMakeLists.txt | 2 +- docs/rel-notes.rst | 18 +++++++++++++++++- src/rmr/common/src/rt_generic_static.c | 12 ++++++++++-- src/rmr/si/src/rtable_si_static.c | 11 +++++++---- test/rt_static_test.c | 19 +++++++++++++------ 6 files changed, 51 insertions(+), 14 deletions(-) diff --git a/CHANGES_CORE.txt b/CHANGES_CORE.txt index d448a57..5d54c19 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.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 --- 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 --- 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 --- 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 --- 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 --- 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, " loading RT from edge case static table\n" ); fprintf( stderr, " %s\n", rt_stuff ); gen_custom_rt( ctx, rt_stuff ); + fprintf( stderr, " 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 -- 2.16.6