* CLI reconnects to backend if backend restarts with a warning

* Note that edits to the candidate database or locks will be lost * To force the CLI to exit if backend restarts, undef `PROTO_RESTART_RECONNECT` * This is an effect of the fix of [Broken pipe error seen in client (cli) when backend restarts and CLICON_SOCK is recreated](#312), the CLI behavior on backend restart is changed. * Added `eof` parameter to `clicon_rpc()` and `clicon_rpc1()` and error handling modified
clicon · Mar 16, 2022 · dfeb7ce · dfeb7ce
1 parent 03f667e
commit dfeb7ce
Show file tree

Hide file tree

Showing 10 changed files with 175 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,11 +37,27 @@
 ## 5.7.0
 Expected: May 2022
 
+### API changes on existing protocol/config features
+
+Users may have to change how they access the system
+
+* CLI reconnects to backend if backend restarts with a warning
+  * Note that edits to the candidate database or locks will be lost
+  * To force the CLI to exit if backend restarts, undef `PROTO_RESTART_RECONNECT`
+  * This is an effect of the fix of [Broken pipe error seen in client (cli) when backend restarts and CLICON_SOCK is recreated](https://github.com/clicon/clixon/issues/312), the CLI behavior on backend restart is changed.
+
 ### Corrected Bugs
 
+* Fixed: [Broken pipe error seen in client (cli) when backend restarts and CLICON_SOCK is recreated](https://github.com/clicon/clixon/issues/312)
 * Fixed: [Xpath API do not support filter data by wildcard](https://github.com/clicon/clixon/issues/313)
 * Fixed: SEGV in cli show yang
 
+### C/CLI-API changes on existing features
+
+Developers may need to change their code
+
+* Added `eof` parameter to `clicon_rpc()` and `clicon_rpc1()` and error handling modified
+
 ## 5.6.0
 8 March 2022
 

diff --git a/apps/cli/cli_main.c b/apps/cli/cli_main.c
@@ -204,11 +204,23 @@ cli_sig_term(int arg)
 
 /*! Setup signal handlers
  */
-static void
+static int
 cli_signal_init (clicon_handle h)
 {
-	cli_signal_block(h);
-	set_signal(SIGTERM, cli_sig_term, NULL);
+    int retval = -1;
+
+    cli_signal_block(h);
+    if (set_signal(SIGTERM, cli_sig_term, NULL) < 0){
+	clicon_err(OE_UNIX, errno, "Setting SIGTERM signal");
+	goto done;
+    }
+    if (set_signal(SIGPIPE, SIG_IGN, NULL) < 0){
+	clicon_err(OE_UNIX, errno, "Setting DIGPIPE signal");
+	goto done;
+    }
+    retval = 0;
+ done:
+    return retval;
 }
 
 /*! Interactive CLI command loop
@@ -653,7 +665,8 @@ main(int    argc,
 	clicon_log_string_limit_set(nr);
 
     /* Setup signal handlers */
-    cli_signal_init(h);
+    if (cli_signal_init(h) < 0)
+	goto done;
 
     /* Backward compatible mode, do not include keys in cgv-arrays in callbacks.
        Should be 0 but default is 1 since all legacy apps use 1

diff --git a/apps/cli/cli_plugin.c b/apps/cli/cli_plugin.c
@@ -606,8 +606,11 @@ clicon_parse(clicon_handle  h,
 	    cli_output_reset();
 	    if (!cligen_exiting(ch)) {	
 		clicon_err_reset();
-		if ((ret = cligen_eval(ch, match_obj, cvv, callbacks)) < 0) 
+		if ((ret = cligen_eval(ch, match_obj, cvv, callbacks)) < 0) {
 		    cli_handler_err(stdout);
+		    if (clicon_suberrno == ESHUTDOWN)
+			goto done;
+		}
 	    }
 	    else
 		ret = 0;

diff --git a/apps/netconf/netconf_main.c b/apps/netconf/netconf_main.c
@@ -602,6 +602,23 @@ netconf_terminate(clicon_handle h)
     return 0;
 }
 
+
+/*! Setup signal handlers
+ */
+static int
+netconf_signal_init (clicon_handle h)
+{
+    int retval = -1;
+
+    if (set_signal(SIGPIPE, SIG_IGN, NULL) < 0){
+	clicon_err(OE_UNIX, errno, "Setting DIGPIPE signal");
+	goto done;
+    }
+    retval = 0;
+ done:
+    return retval;
+}
+
 static int
 timeout_fn(int s,
 	   void *arg)
@@ -804,6 +821,10 @@ main(int    argc,
     if (netconf_module_features(h) < 0)
 	goto done;
 
+    /* Setup signal handlers, int particular PIPE that occurs if backend closes / restarts */
+    if (netconf_signal_init(h) < 0)
+	goto done;
+
     /* Initialize plugin module by creating a handle holding plugin and callback lists */
     if (clixon_plugin_module_init(h) < 0)
 	goto done;

diff --git a/include/clixon_custom.h b/include/clixon_custom.h
@@ -149,3 +149,10 @@
  * See test/fuzz/http1
  */
 #undef RESTCONF_HTTP1_UNITTEST
+
+/*! If backend is restarted, cli and netconf client will retry (once) and reconnect
+ * Note, if client has locked or had edits in progress, these will be lost
+ * A warning will be printed
+ * If not set, client will exit
+ */
+#define PROTO_RESTART_RECONNECT
diff --git a/lib/clixon/clixon_proto.h b/lib/clixon/clixon_proto.h
@@ -79,9 +79,9 @@ int clicon_rpc_connect_inet(clicon_handle         h,
 			    uint16_t              port,
 			    int                  *sock0);
 
-int clicon_rpc(int sock, struct clicon_msg *msg, char **xret);
+int clicon_rpc(int sock, struct clicon_msg *msg, char **xret, int *eof);
 
-int clicon_rpc1(int sock, cbuf *msgin, cbuf *msgret);
+int clicon_rpc1(int sock, cbuf *msgin, cbuf *msgret, int *eof);
 
 int clicon_msg_send(int s, struct clicon_msg *msg);
 

diff --git a/lib/src/clixon_client.c b/lib/src/clixon_client.c
@@ -151,11 +151,12 @@ clixon_client_lock(int         sock,
 		   const int   lock,
 		   const char *db)
 {
-    int                retval = -1;
-    cxobj             *xret = NULL;
-    cxobj             *xd;
-    cbuf              *msg = NULL;
-    cbuf              *msgret = NULL;
+    int    retval = -1;
+    cxobj *xret = NULL;
+    cxobj *xd;
+    cbuf  *msg = NULL;
+    cbuf  *msgret = NULL;
+    int    eof = 0;
 
     clicon_debug(1, "%s", __FUNCTION__);
     if (db == NULL){
@@ -175,8 +176,13 @@ clixon_client_lock(int         sock,
 	    NETCONF_BASE_NAMESPACE,
 	    NETCONF_MESSAGE_ID_ATTR,
 	    lock?"":"un", db, lock?"":"un");
-    if (clicon_rpc1(sock, msg, msgret) < 0)
+    if (clicon_rpc1(sock, msg, msgret, &eof) < 0)
+	goto done;
+    if (eof){
+	close(sock);
+	clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
 	goto done;
+    }
     if (clixon_xml_parse_string(cbuf_get(msgret), YB_NONE, NULL, &xret, NULL) < 0)
 	goto done;
     if ((xd = xpath_first(xret, NULL, "/rpc-reply/rpc-error")) != NULL){
@@ -403,13 +409,14 @@ clixon_client_get_xdata(int         sock,
 			const char *xpath,
 			cxobj     **xdata)
 {
-    int                retval = -1;
-    cxobj             *xret = NULL;
-    cxobj             *xd;
-    cbuf              *msg = NULL;
-    cbuf              *msgret = NULL;
-    const char        *db = "running";
-    cvec              *nsc = NULL; 
+    int          retval = -1;
+    cxobj       *xret = NULL;
+    cxobj       *xd;
+    cbuf        *msg = NULL;
+    cbuf        *msgret = NULL;
+    const char  *db = "running";
+    cvec        *nsc = NULL; 
+    int          eof = 0;
 
     clicon_debug(1, "%s", __FUNCTION__);
     if ((msg = cbuf_new()) == NULL){
@@ -437,8 +444,13 @@ clixon_client_get_xdata(int         sock,
 	cprintf(msg, "/>");
     }
     cprintf(msg, "</get-config></rpc>");
-    if (clicon_rpc1(sock, msg, msgret) < 0)
+    if (clicon_rpc1(sock, msg, msgret, &eof) < 0)
 	goto done;
+    if (eof){
+	close(sock);
+	clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
+	goto done;
+    }
     if (clixon_xml_parse_string(cbuf_get(msgret), YB_NONE, NULL, &xret, NULL) < 0)
 	goto done;
     if ((xd = xpath_first(xret, NULL, "/rpc-reply/rpc-error")) != NULL){

diff --git a/lib/src/clixon_proto.c b/lib/src/clixon_proto.c
@@ -605,42 +605,40 @@ clicon_rpc_connect_inet(clicon_handle      h,
  *
  * TBD: timeout, interrupt?
  * retval may be -1 and
- * errno set to ENOTCONN which means that socket is now closed probably
+ * errno set to ENOTCONN/ESHUTDOWN which means that socket is now closed probably
  * due to remote peer disconnecting. The caller may have to do something,...
  *
  * @param[in]  sock    Socket / file descriptor
  * @param[in]  msg     CLICON msg data structure. It has fixed header and variable body.
  * @param[out] xret    Returned data as netconf xml tree.
- * @retval     0       OK
+ * @param[out] eof     Set if eof encountered
+ * @retval     0       OK (check eof)
  * @retval     -1      Error
  * @see clicon_rpc1 using plain NETCONF XML
  */
 int
 clicon_rpc(int                sock,
 	   struct clicon_msg *msg, 
-	   char             **ret)
+	   char             **ret,
+	   int               *eof)
 {
     int                retval = -1;
     struct clicon_msg *reply = NULL;
-    int                eof;
     char              *data = NULL;
 
     if (clicon_msg_send(sock, msg) < 0)
 	goto done;
-    if (clicon_msg_rcv(sock, &reply, &eof) < 0)
-	goto done;
-    if (eof){
-	clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
-	close(sock); /* assume socket */
-	errno = ESHUTDOWN;
+    if (clicon_msg_rcv(sock, &reply, eof) < 0)
 	goto done;
-    }
+    if (*eof)
+	goto ok;
     data = reply->op_body; /* assume string */
     if (ret && data)
 	if ((*ret = strdup(data)) == NULL){
 	    clicon_err(OE_UNIX, errno, "strdup");
 	    goto done;
 	}
+ ok:
     retval = 0;
   done:
     if (reply)
@@ -655,29 +653,24 @@ clicon_rpc(int                sock,
  * @param[in]  sock    Socket / file descriptor
  * @param[in]  msgin   CLICON msg data structure. It has fixed header and variable body.
  * @param[out] msgret  Returned data as netconf xml tree.
+ * @param[out] eof     Set if eof encountered
  * @retval     0       OK
  * @retval     -1      Error
  * @see clicon_rpc using clicon_msg protocol header
  */
 int
 clicon_rpc1(int   sock,
 	    cbuf *msg,
-	    cbuf *msgret) 
+	    cbuf *msgret,
+	    int  *eof)
 {
     int    retval = -1;
-    int    eof;
 
     clicon_debug(1, "%s", __FUNCTION__);
     if (clicon_msg_send1(sock, msg) < 0)
 	goto done;
-    if (clicon_msg_rcv1(sock, msgret, &eof) < 0)
-	goto done;
-    if (eof){
-	clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
-	close(sock);
-	errno = ESHUTDOWN;
+    if (clicon_msg_rcv1(sock, msgret, eof) < 0)
 	goto done;
-    }
     retval = 0;
   done:
     clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);