Fixing libfcgi bad free bug

Bugfix consist of replacing the magic wrapper instance of os.environ (
aitomatically call os.putenv) by a normal dict instance.

src/pyworker.c

@@ -45,7 +45,7 @@ pid_t spawn(char* py_entrypoint, int wrk_id, int semid, int max_reqs)
 int work(char* py_entrypoint, int wrk_id, int semid, int max_reqs)
 {
 	PyObject *entry_fun, *pystdout_flush, *pystderr_flush,
-	         *py_setenv, *py_clrenv;
+	         *py_osmod;
 	int count, pipe_out[2], pipe_err[2], pipe_ctl[2], err, piper_status;
 	struct sigaction act;
 	sigset_t emptyset;
@@ -62,6 +62,7 @@ int work(char* py_entrypoint, int wrk_id, int semid, int max_reqs)
 	}
 	act.sa_handler = worker_piper_sighandler;
 	act.sa_mask = emptyset;
+	//act.sa_flags = SA_RESTART;
 	act.sa_flags = 0;
 	act.sa_restorer = NULL;
 
@@ -76,7 +77,15 @@ int work(char* py_entrypoint, int wrk_id, int semid, int max_reqs)
 	syslog(	LOG_INFO,
 		"Worker[%d] Python started", wrk_id);
 	
-	get_py_setenv(&py_setenv, &py_clrenv);
+	//importing os
+	py_osmod = PyImport_ImportModule("os");
+	if(!py_osmod)
+	{
+		syslog(LOG_ALERT, "Unable to import os module");
+		log_expt(LOG_ALERT);
+		Py_Exit(EXIT_PYERR);
+	}
+
 	// loading module
 	entry_fun = import_entrypoint(py_entrypoint);
 
@@ -92,6 +101,7 @@ int work(char* py_entrypoint, int wrk_id, int semid, int max_reqs)
 			"Worker[%d] request %d", wrk_id, count);
 		worker_piper_sigrcv = 0;
 		pipe(pipe_ctl); //TODO : check for pipe error
+		//PyOS_BeforeFork();
 		pid_t pid = fork();
 		if(pid < 0)
 		{
@@ -118,7 +128,8 @@ int work(char* py_entrypoint, int wrk_id, int semid, int max_reqs)
 			//printf("Content-type: text/html\r\n\r\nHello world !\n");
 			exit(1);
 		}
-		update_pyenv(py_setenv, py_clrenv);
+		//PyOS_AfterFork_Parent();
+		update_pyenv(py_osmod);
 		//TODO : check if pipe_ctl lock is really needed anymore
 		close(pipe_ctl[1]);
 		PyObject_CallObject(entry_fun, NULL);
@@ -146,7 +157,7 @@ syslog(LOG_DEBUG, "PIPER UNLOCK");
 		syslog(LOG_DEBUG, "Worker[%d] request %d END [OK]",
 			wrk_id, count);
 	}
-	Py_Exit(count == max_reqs ?0:42);
+	Py_Exit(count == max_reqs ?0:EXIT_PYERR);
 }
 
 void worker_piper(int wrk_id, int req_id, int pystdout, int pystderr,
@@ -196,27 +207,23 @@ syslog(LOG_DEBUG, "Worler[%d] req #%d poll_ret = %d", wrk_id, req_id, poll_ret);
 			{
 				syslog(LOG_DEBUG, "Worker[%d] req #%d POLLIN STDOUT !",
 				       wrk_id, req_id);
-				while(1)
+				ret = read(pystdout, buf, PIPE_BUF);
+				syslog(LOG_DEBUG, "Worker[%d] req #%d read(stdout) ret %d",
+				       wrk_id, req_id, ret);
+				if(ret < 0)
 				{
-					ret = read(pystdout, buf, PIPE_BUF);
-					syslog(LOG_DEBUG, "Worker[%d] req #%d read(stdout) ret %d",
-					       wrk_id, req_id, ret);
-					if(ret < 0)
+					err = errno;
+					if(err == EINTR)
 					{
-						err = errno;
-						if(err == EINTR)
-						{
-							continue;
-						}
-						syslog( LOG_ERR,
-						        "Error reading python stdout : %s",
-							strerror(err));
-						exit(err);
+						continue;
 					}
-					buf[ret] = '\0';
-					printf("%s", buf);
-					break;
+					syslog( LOG_ERR,
+						"Error reading python stdout : %s",
+						strerror(err));
+					exit(err);
 				}
+				buf[ret] = '\0';
+				printf("%s", buf);
 			}
 			//TODO handle other poll events
 		}
@@ -262,7 +269,7 @@ syslog(LOG_DEBUG, "Worler[%d] req #%d poll_ret = %d", wrk_id, req_id, poll_ret);
 void worker_piper_sighandler(int signum)
 {
 	worker_piper_sigrcv = 1;
-	syslog(LOG_DEBUG, "SIG");
+	//syslog(LOG_DEBUG, "SIG");
 }
 
 PyObject* import_entrypoint(char* py_entrypoint)
@@ -496,7 +503,6 @@ void update_python_fd(int pipe_out[2], int pipe_err[2])
 				"Unable to fetch os.fdopen() , got NULL.");
 		}
 		err_fmt = NULL;
-		Py_DECREF(os_mod);
 		goto update_python_fd_err_pipes;
 	}
 
@@ -577,17 +583,29 @@ update_python_fd_err:
 }
 
 
-void update_pyenv(PyObject *py_setenv, PyObject *py_clrenv)
+void update_pyenv(PyObject *py_osmod)
 {
-	PyObject *args, *pykey, *pyval, *ret;
+	PyObject *pyenv, *pykey, *pyval;
 	char *key, *value, **cur;
 
 	cur = environ;
 
-	PyObject_CallObject(py_clrenv, NULL); // call os.environ.clear()
+	pyenv = PyObject_GetAttrString(py_osmod, "environ");
+	if(!pyenv)
+	{
+		syslog(LOG_WARNING, "Unable to get os.environ");
+		log_expt(LOG_ALERT);
+	}
+	else
+	{
+		Py_DECREF(pyenv);
+	}
+	pyenv = PyDict_New();
+
+
+
 	while(*cur)
 	{
-		//key = value = strdup(*cur);
 		key = value = *cur;
 		while(*value && *value != '=')
 		{
@@ -613,67 +631,28 @@ syslog(LOG_DEBUG, "PySetEnv '%s'='%s'", key, value);
 			Py_Exit(EXIT_PYERR);
 		}
 		*(value-1) = '='; // **environ restore
-		pyval = PyUnicode_DecodeLocale(value, "surrogateescape");
-		if(!pykey)
+		pyval = PyUnicode_DecodeFSDefault(value);
+		if(!pyval)
 		{
 			syslog(LOG_ALERT, "Unable to parse environ val string '%s'",
 			       value);
 			log_expt(LOG_ALERT);
 			Py_Exit(EXIT_PYERR);
 		}
-		args = Py_BuildValue("OO", pykey, pyval);
+		if(PyDict_SetItem(pyenv, pykey, pyval) == -1)
+		{
+			syslog(LOG_ERR, "Unable to set environ '%s'='%s'",
+			       key, value);
+			log_expt(LOG_ERR);
+		}
 		Py_DECREF(pyval);
 		Py_DECREF(pykey);
 		cur++;
-		ret = PyObject_CallObject(py_setenv, args);
-		if(ret)
-		{
-			Py_DECREF(ret);
-		}
-		if(PyErr_Occurred())
-		{
-			log_expt(LOG_WARNING);
-		}
-		//free(key);
 	}
+	PyObject_SetAttrString(py_osmod, "environ", pyenv);
 
 }
 
-void get_py_setenv(PyObject** pyenv_setitem, PyObject** pyenv_clear)
-{
-	PyObject *osmod, *pyenv;
-	osmod = PyImport_ImportModule("os");
-	if(!osmod)
-	{
-		syslog(LOG_ALERT, "Unable to import os module");
-		log_expt(LOG_ALERT);
-		Py_Exit(EXIT_PYERR);
-	}
-	pyenv = PyObject_GetAttrString(osmod, "environ");
-	if(!pyenv)
-	{
-		syslog(LOG_ALERT, "Unable to get os.environ");
-		log_expt(LOG_ALERT);
-		Py_Exit(EXIT_PYERR);
-	}
-	Py_DECREF(osmod);
-	*pyenv_setitem = PyObject_GetAttrString(pyenv, "__setitem__");
-	if(!*pyenv_setitem)
-	{
-		syslog(LOG_ALERT, "Unable to get os.environ.__setitem__");
-		log_expt(LOG_ALERT);
-		Py_Exit(EXIT_PYERR);
-	}
-	*pyenv_clear = PyObject_GetAttrString(pyenv, "clear");
-	if(!*pyenv_clear)
-	{
-		syslog(LOG_ALERT, "Unable to get os.environ.clear()");
-		log_expt(LOG_ALERT);
-		Py_Exit(EXIT_PYERR);
-	}
-	Py_DECREF(pyenv);
-}
-
 void log_expt(int priority)
 {
 	if(!PyErr_Occurred())

src/pyworker.h

@@ -100,19 +100,14 @@ void update_python_path();
  */
 void update_python_fd(int[2], int[2]);
 
-/**@brief Update python sys.environ using current FCI environ
- * @note For the moment do not delete unset variables only update
- * from environ and add new one
- * @param PyObject* os.environ.__setitem__
- * @param PyObject* os.environ.clean
+/**@brief Clear then update python sys.environ using current FCI environ
+ * @note The environ has to be set without a call to os.putenv, the problem
+ * is that the os.environ is a special mapping calling putenv on setitem...
+ * For these reason the os.environ will be replaced by a new dict instance for
+ * each request...
+ * @param PyObject* os module
  */
-void update_pyenv(PyObject*, PyObject*);
-
-/**@brief Fetch python os.environ.__setitem__ & os.environ.clear()
- * @param PyObject** setitem
- * @param PyObject** clear
- */
-void get_py_setenv(PyObject**, PyObject**);
+void update_pyenv(PyObject*);
 
 void log_expt(int priority);
 

src/responder.c

@@ -31,6 +31,7 @@ int responder_loop(char *py_entrypoint, unsigned int max_reqs,
 	unsigned int n_wrk;
 	int *wrk_pids;
 	int semid, err;
+	int status;
 
 	syslog(LOG_INFO, "Preparing workers");
 
@@ -57,10 +58,13 @@ int responder_loop(char *py_entrypoint, unsigned int max_reqs,
 	//Debug wait & exit
 	for(n_wrk=0; n_wrk != min_wrk; n_wrk++)
 	{
-		wait(NULL);
+		waitpid(wrk_pids[n_wrk], &status, 0);
+		syslog(LOG_DEBUG, "Child %d stopped with status %d",
+		       wrk_pids[n_wrk], status);
 	}
 		//printf("Content-Type: text/html\r\n\r\nHello world !\n");
-	return 0;
+	syslog(LOG_INFO,"Child workers stoped, stopping responder");
+	exit(0);
 }