Internal RESTCONF changes:

- Kill running process directly, not schedul a kill
- fcgi: when kill, make exit more ordered, ensure fcgi-accept returns properly

apps/restconf/restconf_main_fcgi.c

@@ -125,9 +125,14 @@ fcgi_params_set(clicon_handle h,
     return retval;
 }
 
-/* Need global variable to for signal handler XXX */
+/* XXX Need global variable to for SIGCHLD signal handler
+*/
 static clicon_handle _CLICON_HANDLE = NULL;
 
+/* XXX Need global variable to break FCGI accept loop from signal handler see FCGX_Accept_r(req)
+ */
+static int _MYSOCK;
+
 /*! Signall terminates process
  */
 static void
@@ -135,18 +140,21 @@ restconf_sig_term(int arg)
 {
     static int i=0;
 
+    clicon_debug(1, "%s", __FUNCTION__);
     if (i++ == 0)
 	clicon_log(LOG_NOTICE, "%s: %s: pid: %u Signal %d", 
 		   __PROGRAM__, __FUNCTION__, getpid(), arg);
-    else
+    else{
+	clicon_debug(1, "%s done", __FUNCTION__);
 	exit(-1);
-    if (_CLICON_HANDLE){
-	stream_child_freeall(_CLICON_HANDLE);
-	restconf_terminate(_CLICON_HANDLE);
     }
-    clicon_exit_set(); /* checked in clixon_event_loop() */
+
-    clicon_debug(1, "%s done", __FUNCTION__);
+    /* This should ensure no more accepts or incoming packets are processed because next time eventloop
-    exit(-1);
+     * is entered, it will terminate.
+     * However there may be a case of sockets closing rather abruptly for clients
+     */
+    clicon_exit_set(); 
+    close(_MYSOCK);
 }
 
 /*! Reap stream child
@@ -498,6 +506,7 @@ main(int    argc,
 	clicon_err(OE_CFG, errno, "FCGX_OpenSocket");
 	goto done;
     }
+    _MYSOCK = sock;
     /* Change group of fcgi sock fronting reverse proxy to WWWUSER, the effective group is clicon
      * which is backend. */
     gid_t wgid = -1;
@@ -581,6 +590,10 @@ main(int    argc,
 	    goto done;
 	if (finish)
 	    FCGX_Finish_r(req);
+	else if (clicon_exit_get()){
+	    FCGX_Finish_r(req);
+	    break;
+	}
 	else{ /* A handler is forked so we initiate a new request after instead 
 		 of finishing the old */
 	    if (FCGX_InitRequest(req, sock, 0) != 0){
@@ -588,7 +601,6 @@ main(int    argc,
 		goto done;
 	    }
 	}
-	
     } /* while */
     retval = 0;
  done:

apps/restconf/restconf_main_native.c

@@ -1326,7 +1326,6 @@ restconf_native_terminate(clicon_handle h)
     return 0;
 }
 
-
 /*! Query backend of config.
  * Loop to wait for backend starting, try again if not done 
  * @param[out] xrestconf XML restconf config, malloced (if retval = 1)
@@ -1733,6 +1732,10 @@ restconf_sig_term(int arg)
     }
     else
 	exit(-1);
+    /* This should ensure no more accepts or incoming packets are processed because next time eventloop
+     * is entered, it will terminate.
+     * However there may be a case of sockets closing rather abruptly for clients
+     */
     clicon_exit_set(); 
 }
 

lib/src/clixon_proc.c

@@ -597,6 +597,7 @@ clixon_process_operation(clicon_handle  h,
     process_entry_t *pe;
     proc_operation   op;
     int              sched = 0; /* If set, process action should be scheduled, register a timeout */
+    int              isrunning = 0;
     
     clicon_debug(1, "%s name:%s op:%s", __FUNCTION__, name, clicon_int2str(proc_operation_map, op0));
     if (_proc_entry_list == NULL)
@@ -609,12 +610,31 @@ clixon_process_operation(clicon_handle  h,
 	    if (wrapit && pe->pe_callback != NULL)
 		if (pe->pe_callback(h, pe, &op) < 0)
 		    goto done;
-
 	    if (op == PROC_OP_START || op == PROC_OP_STOP || op == PROC_OP_RESTART){
 		pe->pe_operation = op;
 		clicon_debug(1, "%s scheduling name: %s pid:%d op: %s", __FUNCTION__,
 			     name, pe->pe_pid,
 			     clicon_int2str(proc_operation_map, pe->pe_operation));
+		if (pe->pe_state==PROC_STATE_RUNNING &&
+		    (op == PROC_OP_STOP || op == PROC_OP_RESTART)){
+		    isrunning = 0;
+		    if (proc_op_run(pe->pe_pid, &isrunning) < 0)
+			goto done;
+		    if (isrunning) {
+			clicon_log(LOG_NOTICE, "Killing old process %s with pid: %d",
+				   pe->pe_name, pe->pe_pid); /* XXX pid may be 0 */
+			kill(pe->pe_pid, SIGTERM);
+
+		    }
+		    clicon_debug(1, "%s %s(%d) %s --%s--> %s", __FUNCTION__,
+				 pe->pe_name, pe->pe_pid,
+				 clicon_int2str(proc_state_map, pe->pe_state),
+				 clicon_int2str(proc_operation_map, pe->pe_operation),
+				 clicon_int2str(proc_state_map, PROC_STATE_EXITING)
+				 );
+		    pe->pe_state = PROC_STATE_EXITING; /* Keep operation stop/restart */
+		    //		    break;
+               }
 		sched++;
 	    }
 	    else{
@@ -750,6 +770,7 @@ clixon_process_sched(int           fd,
     int              retval = -1;
     process_entry_t *pe;
     int              isrunning; /* Process is actually running */
+    int              sched = 0;
 
     clicon_debug(1, "%s",__FUNCTION__);
     if (_proc_entry_list == NULL)
@@ -762,6 +783,21 @@ clixon_process_sched(int           fd,
 	if (pe->pe_operation != PROC_OP_NONE){
 	    switch (pe->pe_state){
 	    case PROC_STATE_EXITING:
+		switch (pe->pe_operation){
+		case PROC_OP_STOP: 
+		case PROC_OP_RESTART: /* Kill again */
+		    isrunning = 0;
+		    if (proc_op_run(pe->pe_pid, &isrunning) < 0)
+			goto done;		    
+		    if (isrunning) {
+			clicon_log(LOG_NOTICE, "Killing old process %s with pid: %d",
+				   pe->pe_name, pe->pe_pid); /* XXX pid may be 0 */
+			kill(pe->pe_pid, SIGTERM);
+			sched++;
+		    }
+		default:
+		    break;
+		}
 		break; /* only clixon_process_waitpid can change state in exiting */
 	    case PROC_STATE_STOPPED:
 		switch (pe->pe_operation){
@@ -794,24 +830,6 @@ clixon_process_sched(int           fd,
 		if (proc_op_run(pe->pe_pid, &isrunning) < 0)
 		    goto done;
 		switch (pe->pe_operation){
-		case PROC_OP_STOP:
-		    clicon_debug(1, "%s stop pid:%d", __FUNCTION__, pe->pe_pid);
-		case PROC_OP_RESTART:
-		    if (isrunning){
-			clicon_log(LOG_NOTICE, "Killing old process %s with pid: %d", pe->pe_name, pe->pe_pid);
-			kill(pe->pe_pid, SIGTERM);
-			/* Cant wait here because it would block the backend and terminating may involve
-			 * some protocol handling, instead SIGCHLD is receoved and 
-			 * clixon_process_waitpid is called that for waits/reaps the dead process */
-		    }
-		    clicon_debug(1, "%s %s(%d) %s --%s--> %s", __FUNCTION__,
-				 pe->pe_name, pe->pe_pid,
-				 clicon_int2str(proc_state_map, pe->pe_state),
-				 clicon_int2str(proc_operation_map, pe->pe_operation),
-				 clicon_int2str(proc_state_map, PROC_STATE_EXITING)
-				 );
-		    pe->pe_state = PROC_STATE_EXITING; /* Keep operation stop/restart */
-		    break;
 		case PROC_OP_START:
 		    if (isrunning) /* Already runs */
 			break;
@@ -835,6 +853,8 @@ clixon_process_sched(int           fd,
 	}
 	pe = NEXTQ(process_entry_t *, pe);
     } while (pe != _proc_entry_list);
+    if (sched && clixon_process_sched_register(h) < 0)
+	goto done;
  ok:
     retval = 0;
  done:

test/test_datastore.sh

@@ -163,6 +163,7 @@ expectpart "$($clixon_util_datastore $conf lock 756)" 0 ""
 
 # unset conditional parameters 
 unset clixon_util_datastore
+unset ret
 
 rm -rf $mydir
 

test/test_feature.sh

@@ -303,6 +303,8 @@ if [ $BE -ne 0 ]; then
     stop_backend -f $cfg
 fi
 
+unset ret
+
 endtest
 
 rm -rf $dir

test/test_nacm_default.sh

@@ -243,3 +243,4 @@ unset RESTCONFIG
 
 # unset conditional parameters 
 unset format
+unset ret

test/test_nacm_module_read.sh

@@ -272,6 +272,7 @@ fi
 
 # Set by restconf_config
 unset RESTCONFIG
+unset ret
 
 rm -rf $dir
 

test/test_perf_restconf.sh

@@ -249,6 +249,7 @@ unset RESTCONFIG
 unset format
 unset perfnr
 unset perfreq
+unset ret
 
 new "endtest"
 endtest

test/test_restconf.sh

@@ -413,7 +413,7 @@ if [ "${WITH_RESTCONF}" = "native" ]; then
     protos="$protos https"
 fi
 for proto in $protos; do
-#    addrs="127.0.0.1"
+    addrs="127.0.0.1"
     if $IPv6 ; then
 	addrs="$addrs \[::1\]"
     fi
@@ -429,6 +429,7 @@ unset RCPROTO
 # Set by restconf_config
 unset RESTCONFIG
 unset RESTCONFIG1
+unset ret
 
 rm -rf $dir
 

test/test_restconf_internal.sh

@@ -78,7 +78,7 @@ function rpcstatus()
     
     sleep $DEMSLEEP
     new "send rpc status"
-    ret=$($clixon_netconf -qf $cfg<<EOF
+    retx=$($clixon_netconf -qf $cfg<<EOF
 $DEFAULTHELLO
 <rpc $DEFAULTNS>
   <process-control $LIBNS>
@@ -90,14 +90,14 @@ EOF
 )
     # Check pid
     expect="<pid $LIBNS>[0-9]*</pid>"
-    match=$(echo "$ret" | grep --null -Go "$expect")
+    match=$(echo "$retx" | grep --null -Go "$expect")
     if [ -z "$match" ]; then
 	pid=0
     else
 	pid=$(echo "$match" | awk -F'[<>]' '{print $3}')
     fi
     if [ -z "$pid" ]; then
-	err "No pid return value" "$ret"
+	err "No pid return value" "$retx"
     fi
     if $active; then
 	expect="^<rpc-reply $DEFAULTNS><active $LIBNS>$active</active><description $LIBNS>Clixon RESTCONF process</description><command $LIBNS>/www-data/clixon_restconf -f $cfg -D [0-9]</command><status $LIBNS>$status</status><starttime $LIBNS>20[0-9][0-9]\-[0-9][0-9]\-[0-9][0-9]T[0-9][0-9]:[0-9][0-9]:[0-9][0-9]\.[0-9]*Z</starttime><pid $LIBNS>$pid</pid></rpc-reply>]]>]]>$"
@@ -105,9 +105,9 @@ EOF
 	# inactive, no startime or pid
 	expect="^<rpc-reply $DEFAULTNS><active $LIBNS>$active</active><description $LIBNS>Clixon RESTCONF process</description><command $LIBNS>/www-data/clixon_restconf -f $cfg -D [0-9]</command><status $LIBNS>$status</status></rpc-reply>]]>]]>$"
     fi
-    match=$(echo "$ret" | grep --null -Go "$expect")
+    match=$(echo "$retx" | grep --null -Go "$expect")
     if [ -z "$match" ]; then
-	err "$expect" "$ret"
+	err "$expect" "$retx"
     fi
 }
 
@@ -223,7 +223,7 @@ new "4. stop restconf RPC"
 rpcoperation stop
 if [ $? -ne 0 ]; then exit -1; fi
 
-new "Wait for restrconf to stop"
+new "Wait for restconf to stop"
 wait_restconf_stopped
 
 new "5. Get rpc status stopped"
@@ -422,7 +422,7 @@ expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><edit-confi
 new "commit disable"
 expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><commit/></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"
 
-new "17. check status RPC off"
+new "18. check status RPC off"
 rpcstatus false stopped
 if [ $pid -ne 0 ]; then err "Pid" "$pid"; fi
 
@@ -448,16 +448,19 @@ fi
 
 #Start backend -s none should start 
 
-unset pid
+new "kill restconf"
+stop_restconf
+
 sleep $DEMSLEEP # Lots of processes need to die before next test
 
 new "endtest"
 endtest
 
 # Set by restconf_config
+unset pid
 unset RESTCONFIG
 unset RESTCONFDBG
 unset RCPROTO
+unset retx
 
 rm -rf $dir
-

test/test_restconf_internal_usecases.sh

@@ -92,7 +92,7 @@ function rpcstatus()
     
     sleep $DEMSLEEP
     new "send rpc status"
-    ret=$($clixon_netconf -qf $cfg<<EOF
+    retx=$($clixon_netconf -qf $cfg<<EOF
 $DEFAULTHELLO
 <rpc $DEFAULTNS>
   <process-control $LIBNS>
@@ -104,14 +104,14 @@ EOF
 )
     # Check pid
     expect="<pid $LIBNS>[0-9]*</pid>"
-    match=$(echo "$ret" | grep --null -Go "$expect")
+    match=$(echo "$retx" | grep --null -Go "$expect")
     if [ -z "$match" ]; then
 	pid=0
     else
 	pid=$(echo "$match" | awk -F'[<>]' '{print $3}')
     fi
     if [ -z "$pid" ]; then
-	err "No pid return value" "$ret"
+	err "No pid return value" "$retx"
     fi
     if $active; then
 	expect="^<rpc-reply $DEFAULTNS><active $LIBNS>$active</active><description $LIBNS>Clixon RESTCONF process</description><command $LIBNS>/www-data/clixon_restconf -f $cfg -D [0-9]</command><status $LIBNS>$status</status><starttime $LIBNS>20[0-9][0-9]\-[0-9][0-9]\-[0-9][0-9]T[0-9][0-9]:[0-9][0-9]:[0-9][0-9]\.[0-9]*Z</starttime><pid $LIBNS>$pid</pid></rpc-reply>]]>]]>$"
@@ -119,9 +119,9 @@ EOF
 	# inactive, no startime or pid
 	expect="^<rpc-reply $DEFAULTNS><active $LIBNS>$active</active><description $LIBNS>Clixon RESTCONF process</description><command $LIBNS>/www-data/clixon_restconf -f $cfg -D [0-9]</command><status $LIBNS>$status</status></rpc-reply>]]>]]>$"
     fi
-    match=$(echo "$ret" | grep --null -Go "$expect")
+    match=$(echo "$retx" | grep --null -Go "$expect")
     if [ -z "$match" ]; then
-	err "$expect" "$ret"
+	err "$expect" "$retx"
     fi
 }
 
@@ -142,6 +142,7 @@ if [ $BE -ne 0 ]; then
     new "start backend -s init -f $cfg"
     start_backend -s init -f $cfg
 fi
+
 new "wait backend"
 wait_backend
 
@@ -250,9 +251,9 @@ expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><commit/></
 sleep $DEMSLEEP
 new "Check zombies"
 # NOTE unsure where zombies actually appear
-ret=$(ps aux| grep clixon | grep defunc | grep -v grep)
+retx=$(ps aux| grep clixon | grep defunc | grep -v grep)
-if [ -n "$ret" ]; then
+if [ -n "$retx" ]; then
-    err "No zombie process" "$ret"
+    err "No zombie process" "$retx"
 fi
 
 if [ $BE -ne 0 ]; then
@@ -269,9 +270,9 @@ fi
 sleep $DEMSLEEP
 new "Check zombies again"
 # NOTE unsure where zombies actually appear
-ret=$(ps aux| grep clixon | grep defunc | grep -v grep)
+retx=$(ps aux| grep clixon | grep defunc | grep -v grep)
-if [ -n "$ret" ]; then
+if [ -n "$retx" ]; then
-    err "No zombie process" "$ret"
+    err "No zombie process" "$retx"
 fi
 
 # THIRD usecase
@@ -437,6 +438,9 @@ fi
 
 fi # "${WITH_RESTCONF}" != "fcgi"
 
+new "kill restconf"
+stop_restconf
+
 new "endtest"
 endtest
 

test/test_restconf_jukebox.sh

@@ -78,7 +78,7 @@ if [ $BE -ne 0 ]; then
     start_backend -s init -f "$cfg" -- -s
 fi
 
-new "waiting"
+new "wait backend"
 wait_backend
 
 if [ $RC -ne 0 ]; then
@@ -88,10 +88,12 @@ if [ $RC -ne 0 ]; then
     new "start restconf daemon"
     start_restconf -f $cfg
 
-    new "waiting"
+
-    wait_restconf
 fi
 
+new "wait restconf"
+wait_restconf
+    
 new "B.1.1.  Retrieve the Top-Level API Resource root"
 expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/xrd+xml' $RCPROTO://localhost/.well-known/host-meta)" 0 "HTTP/1.1 200 OK" "Content-Type: application/xrd+xml" "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
 

test/test_restconf_op.sh

@@ -229,6 +229,7 @@ fi
 
 # Set by restconf_config
 unset RESTCONFIG
+unset ret
 
 rm -rf $dir
 

test/test_upgrade_failsafe.sh

@@ -367,5 +367,8 @@ fi # valgrindtest
 rm -rf $dir
 
 new "endtest"
+
+unset ret
+
 endtest
 

test/test_xml_trees.sh

@@ -61,11 +61,11 @@ function testrun(){
     x0=$2
     x1=$3
     xp=$4
-    ret=$5
+    retx=$5
     res=$6
 
     echo "$clixon_util_xml_mod -o $op -y $fyang -b "$x0" -x "$x1" -p $xp $OPTS"
-    expectpart "$($clixon_util_xml_mod -o $op -y $fyang -b "$x0" -x "$x1" -p $xp $OPTS)" $ret "$res"
+    expectpart "$($clixon_util_xml_mod -o $op -y $fyang -b "$x0" -x "$x1" -p $xp $OPTS)" $retx "$res"
 }
 
 new "test params: -y $fyang $OPTS"
@@ -125,6 +125,7 @@ rm -rf $dir
 
 # unset conditional parameters 
 unset clixon_util_xml_mod
+unset retx
 
 new "endtest"
 endtest

test/test_yang_default.sh

@@ -126,5 +126,7 @@ testrun
 
 rm -rf $dir
 
+unset ret
+
 new "endtest"
 endtest

test/test_yang_namespace.sh

@@ -149,5 +149,7 @@ unset RESTCONFIG
 
 rm -rf $dir
 
+unset ret
+
 new "endtest"
 endtest