[collectd] patch for SIGSEGV during python plugin configuration in collectd 5.0.0

[Luke Heberling]

Hi List,
I've found that having more than one "Plugin" configuration block for
the python plugin causes a segfault. I first ran into it while trying to
split out some configuration into Includes, but it can be just as easily
reproduced in a single configuration file.

The problem appears to exist because the python plugin unconditionally
initializes itself in the config callback, and if more than one "<Plugin
python>" blocks exist, then the callback is called once for each and
initialization is perfomed repeatedly. My quick-n-dirty "fix" for this
is attached; I simply skip the initialization if it's been done already.
The ModulePath config setting requires some data that's obtained during
the initialization, I sidestepped that problem by rejecting any
ModulePath statements in Plugin config blocks other than the first.

Perhaps there's a better way to go about fixing this problem, but it
does work for me at the moment.

--
Luke Heberling

Backtrace:
#0  0x00007ffff6a37e64 in ?? () from /usr/lib/libpython2.6.so.1.0
#1  0x00007ffff6a9b3ea in PyModule_AddObject () from
/usr/lib/libpython2.6.so.1.0
#2  0x00007ffff6e4106a in cpy_config (ci=0x622200) at python.c:1033
#3  0x00000000004099fd in dispatch_block_plugin (filename=<value
optimized out>) at configfile.c:361
#4  dispatch_block (filename=<value optimized out>) at configfile.c:389
#5  cf_read (filename=<value optimized out>) at configfile.c:942
#6  0x0000000000405f65 in main (argc=4, argv=0x7fffffffe768) at
collectd.c:477


Configuration:
LoadPlugin python
<Plugin python>
        LogTraces false
</Plugin>
<Plugin python>
        LogTraces false
</Plugin>
<Plugin python>
        LogTraces false
</Plugin>

diff --git a/src/python.c b/src/python.c
index ee67388..132e8e7 100644
--- a/src/python.c
+++ b/src/python.c
@@ -980,8 +980,10 @@ static int cpy_config(oconfig_item_t *ci) {
 	int i;
 	char *argv = "";
 	PyObject *sys, *tb;
-	PyObject *sys_path;
+	PyObject *sys_path = NULL;
 	PyObject *module;
+
+	if (!Py_IsInitialized()) {
 	
 	/* Ok in theory we shouldn't do initialization at this point
 	 * but we have to. In order to give python scripts a chance
@@ -1039,6 +1041,8 @@ static int cpy_config(oconfig_item_t *ci) {
 	PyModule_AddIntConstant(module, "NOTIF_FAILURE", NOTIF_FAILURE);
 	PyModule_AddIntConstant(module, "NOTIF_WARNING", NOTIF_WARNING);
 	PyModule_AddIntConstant(module, "NOTIF_OKAY", NOTIF_OKAY);
+	}
+
 	for (i = 0; i < ci->children_num; ++i) {
 		oconfig_item_t *item = ci->children + i;
 		
@@ -1072,6 +1076,10 @@ static int cpy_config(oconfig_item_t *ci) {
 			if (cpy_format_exception == NULL)
 				cpy_log_exception("python initialization");
 		} else if (strcasecmp(item->key, "ModulePath") == 0) {
+			if (sys_path == NULL) {
+				ERROR("python plugin: ModulePath must be in the first <Plugin python> block");
+				continue;
+			}
 			char *dir = NULL;
 			PyObject *dir_object;
 			
@@ -1138,7 +1146,7 @@ static int cpy_config(oconfig_item_t *ci) {
 			WARNING("python plugin: Ignoring unknown config key \"%s\".", item->key);
 		}
 	}
-	Py_DECREF(sys_path);
+	if (sys_path != NULL) Py_DECREF(sys_path);
 	return 0;
 }
 

_______________________________________________
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd