Fork fcgi handler for streams

CHANGELOG.md

@@ -28,6 +28,7 @@
 * Restconf stream notification support according to RFC8040
   * See (apps/restconf/README.md) for more details.
   * start-time and stop-time query parameters
+  * Fork fcgi handler for streams
   * Set access/subscribe base URL with: CLICON_STREAM_URL (default "https://localhost") and CLICON_STREAM_PATH (default "streams")
     * Example: new stream "foo" will get access URL: https://localhost/streams/foo
   * Alternative variant using pub/sub support enabled by ./configure --enable-publish

apps/restconf/restconf_lib.c

@@ -54,7 +54,7 @@
 /* clicon */
 #include <clixon/clixon.h>
 
-#include <fcgi_stdio.h> /* Need to be after clixon_xml-h due to attribute format */
+#include <fcgiapp.h> /* Need to be after clixon_xml-h due to attribute format */
 
 #include "restconf_lib.h"
 
@@ -485,3 +485,23 @@ api_return_err(clicon_handle h,
         cbuf_free(cb);
     return retval;
 }
+
+int
+restconf_terminate(clicon_handle h)
+{
+    yang_spec *yspec;
+    cxobj     *x;
+
+    clixon_plugin_exit(h);
+    rpc_callback_delete_all();
+    clicon_rpc_close_session(h);
+    if ((yspec = clicon_dbspec_yang(h)) != NULL)
+	yspec_free(yspec);
+    if ((yspec = clicon_config_yang(h)) != NULL)
+	yspec_free(yspec);
+    if ((x = clicon_conf_xml(h)) != NULL)
+	xml_free(x);
+    clicon_handle_exit(h);
+    clicon_log_exit();
+    return 0;
+}

apps/restconf/restconf_lib.h

@@ -62,5 +62,6 @@ cbuf *readdata(FCGX_Request *r);
 int get_user_cookie(char *cookiestr, char  *attribute, char **val);
 int api_return_err(clicon_handle h, FCGX_Request *r, cxobj *xerr,
 		   int pretty, int use_xml);
+int restconf_terminate(clicon_handle h);
 
 #endif /* _RESTCONF_LIB_H_ */

apps/restconf/restconf_main.c

@@ -73,7 +73,7 @@
 /* clicon */
 #include <clixon/clixon.h>
 
-#include <fcgi_stdio.h> /* Need to be after clixon_xml.h due to attribute format */
+#include <fcgiapp.h> /* Need to be after clixon_xml.h due to attribute format */
 
 /* restconf */
 #include "restconf_lib.h"
@@ -443,27 +443,8 @@ api_restconf(clicon_handle h,
     return retval;
 }
 
-static int
-restconf_terminate(clicon_handle h)
-{
-    yang_spec *yspec;
-    cxobj     *x;
 
-    clixon_plugin_exit(h);
+/* Need global variable to for signal handler XXX */
-    rpc_callback_delete_all();
-    clicon_rpc_close_session(h);
-    if ((yspec = clicon_dbspec_yang(h)) != NULL)
-	yspec_free(yspec);
-    if ((yspec = clicon_config_yang(h)) != NULL)
-	yspec_free(yspec);
-    if ((x = clicon_conf_xml(h)) != NULL)
-	xml_free(x);
-    clicon_handle_exit(h);
-    clicon_log_exit();
-    return 0;
-}
-
-/* Need global variable to for signal handler */
 static clicon_handle _CLICON_HANDLE = NULL;
 
 /*! Signall terminates process
@@ -478,12 +459,24 @@ restconf_sig_term(int arg)
 		   __PROGRAM__, __FUNCTION__, getpid(), arg);
     else
 	exit(-1);
-    if (_CLICON_HANDLE)
+    if (_CLICON_HANDLE){
+	stream_child_freeall(_CLICON_HANDLE);
 	restconf_terminate(_CLICON_HANDLE);
+    }
     clicon_exit_set(); /* checked in event_loop() */
     exit(-1);
 }
 
+static void
+restconf_sig_child(int arg)
+{
+    int status;
+    int pid;
+
+    if ((pid = waitpid(-1, &status, 0)) != -1 && WIFEXITED(status))
+	stream_child_free(_CLICON_HANDLE, pid);
+}
+
 /*! Usage help routine
  * @param[in]  argv0  command line
  * @param[in]  h      Clicon handle
@@ -532,6 +525,7 @@ main(int    argc,
     yang_spec   *yspecfg = NULL; /* For config XXX clixon bug */
     char        *yang_filename = NULL;
     char        *stream_path;
+    int          finish;
     
     /* In the startup, logs to stderr & debug flag set later */
     clicon_log_init(__PROGRAM__, LOG_INFO, logdst); 
@@ -579,6 +573,11 @@ main(int    argc,
 	clicon_err(OE_DEMON, errno, "Setting signal");
 	goto done;
     }
+    if (set_signal(SIGCHLD, restconf_sig_child, NULL) < 0){
+	clicon_err(OE_DEMON, errno, "Setting signal");
+	goto done;
+    }
+
     /* Create configure yang-spec */
     if ((yspecfg = yspec_new()) == NULL)
 	goto done;
@@ -663,11 +662,12 @@ main(int    argc,
     clixon_plugin_start(h, argc+1, argv-1);
     *(argv-1) = tmp;
 
+    /**/
     if ((sockpath = clicon_option_str(h, "CLICON_RESTCONF_PATH")) == NULL){
 	clicon_err(OE_CFG, errno, "No CLICON_RESTCONF_PATH in clixon configure file");
 	goto done;
     }
-    if (FCGX_Init() != 0){
+    if (FCGX_Init() != 0){ /* How to cleanup memory after this? */
 	clicon_err(OE_CFG, errno, "FCGX_Init");
 	goto done;
     }
@@ -676,12 +676,13 @@ main(int    argc,
 	clicon_err(OE_CFG, errno, "FCGX_OpenSocket");
 	goto done;
     }
-
     if (FCGX_InitRequest(r, sock, 0) != 0){
 	clicon_err(OE_CFG, errno, "FCGX_InitRequest");
 	goto done;
     }
     while (1) {
+	finish = 1; /* If zero, dont finish request, initiate new */
+
 	if (FCGX_Accept_r(r) < 0) {
 	    clicon_err(OE_CFG, errno, "FCGX_Accept_r");
 	    goto done;
@@ -692,7 +693,7 @@ main(int    argc,
 	    if (strncmp(path, "/" RESTCONF_API, strlen("/" RESTCONF_API)) == 0)
 		api_restconf(h, r); /* This is the function */
 	    else if (strncmp(path+1, stream_path, strlen(stream_path)) == 0) {
-		api_stream(h, r, stream_path); 
+		api_stream(h, r, stream_path, &finish); 
 	    }
 	    else if (strncmp(path, RESTCONF_WELL_KNOWN, strlen(RESTCONF_WELL_KNOWN)) == 0) {
 		api_well_known(h, r); /*  */
@@ -704,10 +705,19 @@ main(int    argc,
 	}
 	else
 	    clicon_debug(1, "NULL URI");
+	if (finish)
 	    FCGX_Finish_r(r);
+	else{ /* A handler is forked so we initiate a new request after instead 
+		 of finnishing the old */
+	    if (FCGX_InitRequest(r, sock, 0) != 0){
+		clicon_err(OE_CFG, errno, "FCGX_InitRequest");
+		goto done;
+	    }
+	}
     }
     retval = 0;
  done:
+    stream_child_freeall(h);
     restconf_terminate(h);
     return retval;
 }

apps/restconf/restconf_methods.c

@@ -118,7 +118,7 @@ Mapping netconf error-tag -> status code
 /* clicon */
 #include <clixon/clixon.h>
 
-#include <fcgi_stdio.h> /* Need to be after clixon_xml-h due to attribute format */
+#include <fcgiapp.h> /* Need to be after clixon_xml-h due to attribute format */
 
 #include "restconf_lib.h"
 #include "restconf_methods.h"

apps/restconf/restconf_stream.c

@@ -82,11 +82,71 @@
 /* clicon */
 #include <clixon/clixon.h>
 
-#include <fcgi_stdio.h> /* Need to be after clixon_xml.h due to attribute format */
+#include <fcgiapp.h> /* Need to be after clixon_xml.h due to attribute format */
 
 #include "restconf_lib.h"
 #include "restconf_stream.h"
 
+/*
+ * Constants
+ */
+/* Enable for forking stream subscription loop. 
+ * Disable to get single threading but blocking on streams
+ */
+#define STREAM_FORK 1
+
+/* Keep track of children - whjen they exit - their FCGX handle needs to be 
+ * freed with  FCGX_Free(&rbk, 0);
+ */
+struct stream_child{
+    qelem_t                     sc_q;   /* queue header */
+    int                         sc_pid; /* Child process id */
+    FCGX_Request                sc_r;   /* FCGI stream data */
+};
+/* Linked list of children
+ * @note could hang STREAM_CHILD list on clicon handle instead.
+ */
+static struct stream_child *STREAM_CHILD = NULL; 
+
+/*! Find restconf child using PID and cleanup FCGI Request data
+ * @param[in]  h   Clicon handle
+ * @param[in]  pid Process id of child
+ * @note could hang STREAM_CHILD list on clicon handle instead.
+ */
+int
+stream_child_free(clicon_handle h,
+		  int           pid)
+{
+    struct stream_child *sc;
+    
+    if ((sc = STREAM_CHILD) != NULL){
+	do {
+	    if (pid == sc->sc_pid){
+		DELQ(sc, STREAM_CHILD, struct stream_child *);
+		FCGX_Free(&sc->sc_r, 0);
+		free(sc);
+		goto done;
+	    }
+	    sc = NEXTQ(struct stream_child *, sc);
+	} while (sc && sc !=  STREAM_CHILD);
+    }
+ done:
+    return 0;
+}
+
+int
+stream_child_freeall(clicon_handle h)
+{
+    struct stream_child *sc;
+
+    while ((sc = STREAM_CHILD) != NULL){
+	DELQ(sc, STREAM_CHILD, struct stream_child *);
+	FCGX_Free(&sc->sc_r, 1);
+	free(sc);
+    }
+    return 0;
+}
+
 /*! Callback when stream notifications arrive from backend
  */
 static int
@@ -278,7 +338,8 @@ stream_timeout(int   s,
 int
 api_stream(clicon_handle h,
 	   FCGX_Request *r,
-	   char         *streampath)
+	   char         *streampath,
+	   int          *finish)
 {
     int    retval = -1;
     char  *path;
@@ -298,6 +359,10 @@ api_stream(clicon_handle h,
     cxobj *xret = NULL;
     cxobj *xerr;
     int    s=-1;
+#ifdef STREAM_FORK
+    int    pid;
+    struct stream_child *sc;
+#endif
 
     clicon_debug(1, "%s", __FUNCTION__);
     path = FCGX_GetParam("DOCUMENT_URI", r->envp);
@@ -362,6 +427,23 @@ api_stream(clicon_handle h,
     if (restconf_stream(h, r, method, qvec, pretty, use_xml, &s) < 0)
 	goto done;
     if (s != -1){
+#ifdef STREAM_FORK
+	if ((pid = fork()) == 0){ /* child */
+	    if (pvec)
+		free(pvec);
+	    if (dvec)
+		cvec_free(dvec);
+	    if (qvec)
+		cvec_free(qvec);
+	    if (pcvec)
+		cvec_free(pcvec);
+	    if (cb)
+		cbuf_free(cb);
+	    if (cbret)
+		cbuf_free(cbret);
+	    if (xret)
+		xml_free(xret);
+#endif /* STREAM_FORK */
 	    /* Listen to backend socket */
 	    if (event_reg_fd(s, 
 			     restconf_stream_cb, 
@@ -379,7 +461,30 @@ api_stream(clicon_handle h,
 	    event_loop();
 	    close(s);
 	    event_unreg_fd(s, restconf_stream_cb);
+	    event_unreg_fd(r->listen_sock, restconf_stream_cb);
+	    event_unreg_timeout(stream_timeout, (void*)r);
 	    clicon_exit_reset();
+#ifdef STREAM_FORK
+	    FCGX_Finish_r(r);
+	    FCGX_Free(r, 0);	    
+	    fprintf(stderr, "child exit and free\n");
+	    restconf_terminate(h);
+	    exit(0);
+	}
+	/* parent */
+	/* Create stream_child struct and store pid and FCGI data, when child
+	 * killed, call FCGX_Free
+	 */
+	if ((sc = malloc(sizeof(struct stream_child))) == NULL){
+	    clicon_err(OE_XML, errno, "malloc");
+	    goto done;
+	}
+	memset(sc, 0, sizeof(struct stream_child));
+	sc->sc_pid = pid;
+	sc->sc_r = *r;
+	ADDQ(sc, STREAM_CHILD);
+	*finish = 0; /* If spawn child, we should not finish this stream */
+#endif /* STREAM_FORK */
     }
  ok:
     retval = 0;
@@ -399,6 +504,5 @@ api_stream(clicon_handle h,
 	cbuf_free(cbret);
     if (xret)
 	xml_free(xret);
-
     return retval;
 }

apps/restconf/restconf_stream.h

@@ -39,6 +39,8 @@
 /*
  * Prototypes
  */
-int api_stream(clicon_handle h, FCGX_Request *r, char *streampath);
+int stream_child_free(clicon_handle h, int pid);
+int stream_child_freeall(clicon_handle h);
+int api_stream(clicon_handle h, FCGX_Request *r, char *streampath, int *finish);
 
 #endif /* _RESTCONF_STREAM_H_ */

example/example_restconf.c

@@ -39,7 +39,7 @@
 #include <errno.h>
 #include <ctype.h>
 #include <signal.h>
-#include <fcgi_stdio.h>
+#include <fcgiapp.h>
 #include <curl/curl.h>
 
 /* cligen */

test/lib.sh

@@ -18,7 +18,7 @@ clixon_netconf=clixon_netconf
 
 # How to run restconf stand-alone and using valgrind
 #sudo su -c "/www-data/clixon_restconf -f $cfg -D 1" -s /bin/sh www-data
-#sudo su -c "valgrind --leak-check=full --show-leak-kinds=all /www-data/clixon_restconf -f $cfg -D 1" -s /bin/sh www-data
+#sudo su -c "valgrind --trace-children=no --child-silent-after-fork=yes --leak-check=full --show-leak-kinds=all /www-data/clixon_restconf -f $cfg -D 1" -s /bin/sh www-data
 
 #clixon_backend="valgrind --leak-check=full --show-leak-kinds=all clixon_backend"
 clixon_backend=clixon_backend

test/test_stream.sh

@@ -111,7 +111,8 @@ new "kill old restconf daemon"
 sudo pkill -u www-data clixon_restconf
       
 new "start restconf daemon"
-sudo start-stop-daemon -S -q -o -b -x /www-data/clixon_restconf -d /www-data -c www-data -- -f $cfg  -y $fyang # -D 1
+
+sudo su -c "/www-data/clixon_restconf -f $cfg -y $fyang" -s /bin/sh www-data &
 
 sleep 2
 
@@ -167,7 +168,6 @@ sleep 2
 new "restconf monitor event nonexist stream"
 expectwait 'curl -s -X GET -H "Accept: text/event-stream" -H "Cache-Control: no-cache" -H "Connection: keep-alive" http://localhost/streams/NOTEXIST' 0 '<errors xmlns="urn:ietf:params:xml:ns:yang:ietf-restconf"><error><error-tag>invalid-value</error-tag><error-type>application</error-type><error-severity>error</error-severity><error-message>No such stream</error-message></error></errors>' 2
 
-
 # 2a) start subscription 8s - expect 1-2 notifications
 new "2a) start subscriptions 8s - expect 1-2 notifications"
 ret=$($UTIL -u http://localhost/streams/EXAMPLE -t 8)
@@ -236,6 +236,23 @@ if [ $nr -lt 10 -o $nr -gt 14 ]; then
     err 10 "$nr"
 fi
 
+# Try parallell
+# start background job
+curl -s -X GET  -H "Accept: text/event-stream" -H "Cache-Control: no-cache" -H "Connection: keep-alive" "http://localhost/streams/EXAMPLE" > /dev/null &
+
+new "Start subscription in parallell"
+ret=$($UTIL -u http://localhost/streams/EXAMPLE -t 8)
+expect="data: <notification xmlns=\"urn:ietf:params:xml:ns:netconf:notification:1.0\"><eventTime>${DATE}T[0-9:.]*</eventTime><event><event-class>fault</event-class><reportingEntity><card>Ethernet0</card></reportingEntity><severity>major</severity></event>"
+
+match=$(echo "$ret" | grep -Eo "$expect")
+if [ -z "$match" ]; then
+    err "$expect" "$ret"
+fi
+nr=$(echo "$ret" | grep -c "data:")
+if [ $nr -lt 1 -o $nr -gt 2 ]; then
+    err 2 "$nr"
+fi
+
 #-----------------
 
 sudo pkill -u www-data clixon_restconf