fixed incomplete code in avl.c; added comments about the status of appconfig.c

4 files changed, 208 insertions, 138 deletions

diff --git a/src/appconfig.c b/src/appconfig.c
index b76c4f26c2..c164f232e0 100644
--- a/src/appconfig.c
+++ b/src/appconfig.c
@@ -1,3 +1,20 @@
+
+/*
+ * TODO
+ *
+ * 1. Re-write this using DICTIONARY
+ *
+ *    FIXME
+ *    The way it is now, concurrency locking is incomplete!
+ *    It makes sure that only one writes to the structures
+ *    but at the same time there may be unlimited readers.
+ *    This can cause crashes.
+ *
+ *    Of course, rewriting this to use DICTIONARY instead of
+ *    directly accessing AVL structures, will solve the problem.
+ *
+ */
+
 #ifdef HAVE_CONFIG_H
 #include <config.h>
 #endif
@@ -54,7 +71,35 @@ struct config {
 
 
 // ----------------------------------------------------------------------------
-// config value
+// locking
+
+static inline void config_global_read_lock(void) {
+	pthread_rwlock_rdlock(&config_rwlock);
+}
+
+static inline void config_global_write_lock(void) {
+	pthread_rwlock_wrlock(&config_rwlock);
+}
+
+static inline void config_global_unlock(void) {
+	pthread_rwlock_unlock(&config_rwlock);
+}
+
+static inline void config_section_read_lock(struct config *co) {
+	pthread_rwlock_rdlock(&co->rwlock);
+}
+
+static inline void config_section_write_lock(struct config *co) {
+	pthread_rwlock_wrlock(&co->rwlock);
+}
+
+static inline void config_section_unlock(struct config *co) {
+	pthread_rwlock_unlock(&co->rwlock);
+}
+
+
+// ----------------------------------------------------------------------------
+// config name-value index
 
 static int config_value_iterator(avl *a) { if(a) {}; return 0; }
 
@@ -76,8 +121,9 @@ static struct config_value *config_value_index_find(struct config *co, const cha
 	return result;
 }
 
+
 // ----------------------------------------------------------------------------
-// config
+// config sections index
 
 static int config_iterator(avl *a) { if(a) {}; return 0; }
 
@@ -104,39 +150,15 @@ static struct config *config_index_find(const char *name, uint32_t hash) {
 	return result;
 }
 
-struct config_value *config_value_create(struct config *co, const char *name, const char *value)
-{
-	debug(D_CONFIG, "Creating config entry for name '%s', value '%s', in section '%s'.", name, value, co->name);
-
-	struct config_value *cv = calloc(1, sizeof(struct config_value));
-	if(!cv) fatal("Cannot allocate config_value");
 
-	cv->name = strdup(name);
-	if(!cv->name) fatal("Cannot allocate config.name");
-	cv->hash = simple_hash(cv->name);
-
-	cv->value = strdup(value);
-	if(!cv->value) fatal("Cannot allocate config.value");
-
-	config_value_index_add(co, cv);
-
-	// no need for string termination, due to calloc()
-
-	pthread_rwlock_wrlock(&co->rwlock);
-
-	struct config_value *cv2 = co->values;
-	if(cv2) {
-		while (cv2->next) cv2 = cv2->next;
-		cv2->next = cv;
-	}
-	else co->values = cv;
-
-	pthread_rwlock_unlock(&co->rwlock);
+// ----------------------------------------------------------------------------
+// config section methods
 
-	return cv;
+static inline struct config *config_section_find(const char *section) {
+	return config_index_find(section, 0);
 }
 
-struct config *config_create(const char *section)
+static inline struct config *config_section_create(const char *section)
 {
 	debug(D_CONFIG, "Creating section '%s'.", section);
 
@@ -147,14 +169,11 @@ struct config *config_create(const char *section)
 	if(!co->name) fatal("Cannot allocate config.name");
 	co->hash = simple_hash(co->name);
 
-	pthread_rwlock_init(&co->rwlock, NULL);
 	avl_init_lock(&co->values_index, config_value_compare);
 
-	config_index_add(co);
-
-	// no need for string termination, due to calloc()
+	config_global_write_lock();
 
-	pthread_rwlock_wrlock(&config_rwlock);
+	config_index_add(co);
 
 	struct config *co2 = config_root;
 	if(co2) {
@@ -163,98 +182,43 @@ struct config *config_create(const char *section)
 	}
 	else config_root = co;
 
-	pthread_rwlock_unlock(&config_rwlock);
+	config_global_unlock();
 
 	return co;
 }
 
-struct config *config_find_section(const char *section)
-{
-	return config_index_find(section, 0);
-}
 
-int load_config(char *filename, int overwrite_used)
-{
-	int line = 0;
-	struct config *co = NULL;
-
-	char buffer[CONFIG_FILE_LINE_MAX + 1], *s;
-
-	if(!filename) filename = CONFIG_DIR "/" CONFIG_FILENAME;
-	FILE *fp = fopen(filename, "r");
-	if(!fp) {
-		error("Cannot open file '%s'", filename);
-		return 0;
-	}
-
-	while(fgets(buffer, CONFIG_FILE_LINE_MAX, fp) != NULL) {
-		buffer[CONFIG_FILE_LINE_MAX] = '\0';
-		line++;
-
-		s = trim(buffer);
-		if(!s) {
-			debug(D_CONFIG, "Ignoring line %d, it is empty.", line);
-			continue;
-		}
-
-		int len = (int) strlen(s);
-		if(*s == '[' && s[len - 1] == ']') {
-			// new section
-			s[len - 1] = '\0';
-			s++;
+// ----------------------------------------------------------------------------
+// config name-value methods
 
-			co = config_find_section(s);
-			if(!co) co = config_create(s);
+static inline struct config_value *config_value_create(struct config *co, const char *name, const char *value)
+{
+	debug(D_CONFIG, "Creating config entry for name '%s', value '%s', in section '%s'.", name, value, co->name);
 
-			continue;
-		}
+	struct config_value *cv = calloc(1, sizeof(struct config_value));
+	if(!cv) fatal("Cannot allocate config_value");
 
-		if(!co) {
-			// line outside a section
-			error("Ignoring line %d ('%s'), it is outside all sections.", line, s);
-			continue;
-		}
+	cv->name = strdup(name);
+	if(!cv->name) fatal("Cannot allocate config.name");
+	cv->hash = simple_hash(cv->name);
 
-		char *name = s;
-		char *value = strchr(s, '=');
-		if(!value) {
-			error("Ignoring line %d ('%s'), there is no = in it.", line, s);
-			continue;
-		}
-		*value = '\0';
-		value++;
+	cv->value = strdup(value);
+	if(!cv->value) fatal("Cannot allocate config.value");
 
-		name = trim(name);
-		value = trim(value);
+	config_section_write_lock(co);
 
-		if(!name) {
-			error("Ignoring line %d, name is empty.", line);
-			continue;
-		}
-		if(!value) {
-			debug(D_CONFIG, "Ignoring line %d, value is empty.", line);
-			continue;
-		}
-
-		struct config_value *cv = config_value_index_find(co, name, 0);
+	config_value_index_add(co, cv);
 
-		if(!cv) cv = config_value_create(co, name, value);
-		else {
-			if(((cv->flags & CONFIG_VALUE_USED) && overwrite_used) || !(cv->flags & CONFIG_VALUE_USED)) {
-				debug(D_CONFIG, "Overwriting '%s/%s'.", line, co->name, cv->name);
-				free(cv->value);
-				cv->value = strdup(value);
-				if(!cv->value) fatal("Cannot allocate config.value");
-			}
-			else
-				debug(D_CONFIG, "Ignoring line %d, '%s/%s' is already present and used.", line, co->name, cv->name);
-		}
-		cv->flags |= CONFIG_VALUE_LOADED;
+	struct config_value *cv2 = co->values;
+	if(cv2) {
+		while (cv2->next) cv2 = cv2->next;
+		cv2->next = cv;
 	}
+	else co->values = cv;
 
-	fclose(fp);
+	config_section_unlock(co);
 
-	return 1;
+	return cv;
 }
 
 char *config_get(const char *section, const char *name, const char *default_value)
@@ -263,8 +227,8 @@ char *config_get(const char *section, const char *name, const char *default_valu
 
 	debug(D_CONFIG, "request to get config in section '%s', name '%s', default_value '%s'", section, name, default_value);
 
-	struct config *co = config_find_section(section);
-	if(!co) co = config_create(section);
+	struct config *co = config_section_find(section);
+	if(!co) co = config_section_create(section);
 
 	cv = config_value_index_find(co, name, 0);
 	if(!cv) {
@@ -341,7 +305,7 @@ const char *config_set_default(const char *section, const char *name, const char
 
 	debug(D_CONFIG, "request to set config in section '%s', name '%s', value '%s'", section, name, value);
 
-	struct config *co = config_find_section(section);
+	struct config *co = config_section_find(section);
 	if(!co) return config_set(section, name, value);
 
 	cv = config_value_index_find(co, name, 0);
@@ -369,8 +333,8 @@ const char *config_set(const char *section, const char *name, const char *value)
 
 	debug(D_CONFIG, "request to set config in section '%s', name '%s', value '%s'", section, name, value);
 
-	struct config *co = config_find_section(section);
-	if(!co) co = config_create(section);
+	struct config *co = config_section_find(section);
+	if(!co) co = config_section_create(section);
 
 	cv = config_value_index_find(co, name, 0);
 	if(!cv) cv = config_value_create(co, name, value);
@@ -408,6 +372,94 @@ int config_set_boolean(const char *section, const char *name, int value)
 	return value;
 }
 
+
+// ----------------------------------------------------------------------------
+// config load/save
+
+int load_config(char *filename, int overwrite_used)
+{
+	int line = 0;
+	struct config *co = NULL;
+
+	char buffer[CONFIG_FILE_LINE_MAX + 1], *s;
+
+	if(!filename) filename = CONFIG_DIR "/" CONFIG_FILENAME;
+	FILE *fp = fopen(filename, "r");
+	if(!fp) {
+		error("Cannot open file '%s'", filename);
+		return 0;
+	}
+
+	while(fgets(buffer, CONFIG_FILE_LINE_MAX, fp) != NULL) {
+		buffer[CONFIG_FILE_LINE_MAX] = '\0';
+		line++;
+
+		s = trim(buffer);
+		if(!s) {
+			debug(D_CONFIG, "Ignoring line %d, it is empty.", line);
+			continue;
+		}
+
+		int len = (int) strlen(s);
+		if(*s == '[' && s[len - 1] == ']') {
+			// new section
+			s[len - 1] = '\0';
+			s++;
+
+			co = config_section_find(s);
+			if(!co) co = config_section_create(s);
+
+			continue;
+		}
+
+		if(!co) {
+			// line outside a section
+			error("Ignoring line %d ('%s'), it is outside all sections.", line, s);
+			continue;
+		}
+
+		char *name = s;
+		char *value = strchr(s, '=');
+		if(!value) {
+			error("Ignoring line %d ('%s'), there is no = in it.", line, s);
+			continue;
+		}
+		*value = '\0';
+		value++;
+
+		name = trim(name);
+		value = trim(value);
+
+		if(!name) {
+			error("Ignoring line %d, name is empty.", line);
+			continue;
+		}
+		if(!value) {
+			debug(D_CONFIG, "Ignoring line %d, value is empty.", line);
+			continue;
+		}
+
+		struct config_value *cv = config_value_index_find(co, name, 0);
+
+		if(!cv) cv = config_value_create(co, name, value);
+		else {
+			if(((cv->flags & CONFIG_VALUE_USED) && overwrite_used) || !(cv->flags & CONFIG_VALUE_USED)) {
+				debug(D_CONFIG, "Overwriting '%s/%s'.", line, co->name, cv->name);
+				free(cv->value);
+				cv->value = strdup(value);
+				if(!cv->value) fatal("Cannot allocate config.value");
+			}
+			else
+				debug(D_CONFIG, "Ignoring line %d, '%s/%s' is already present and used.", line, co->name, cv->name);
+		}
+		cv->flags |= CONFIG_VALUE_LOADED;
+	}
+
+	fclose(fp);
+
+	return 1;
+}
+
 void generate_config(BUFFER *wb, int only_changed)
 {
 	int i, pri;
@@ -433,7 +485,7 @@ void generate_config(BUFFER *wb, int only_changed)
 				break;
 		}
 
-		pthread_rwlock_wrlock(&config_rwlock);
+		config_global_write_lock();
 		for(co = config_root; co ; co = co->next) {
 			if(strcmp(co->name, "global") == 0 || strcmp(co->name, "plugins") == 0 || strcmp(co->name, "registry") == 0) pri = 0;
 			else if(strncmp(co->name, "plugin:", 7) == 0) pri = 1;
@@ -444,15 +496,13 @@ void generate_config(BUFFER *wb, int only_changed)
 				int changed = 0;
 				int count = 0;
 
-				pthread_rwlock_wrlock(&co->rwlock);
-
+				config_section_write_lock(co);
 				for(cv = co->values; cv ; cv = cv->next) {
 					used += (cv->flags & CONFIG_VALUE_USED)?1:0;
 					changed += (cv->flags & CONFIG_VALUE_CHANGED)?1:0;
 					count++;
 				}
-
-				pthread_rwlock_unlock(&co->rwlock);
+				config_section_unlock(co);
 
 				if(!count) continue;
 				if(only_changed && !changed) continue;
@@ -463,7 +513,7 @@ void generate_config(BUFFER *wb, int only_changed)
 
 				buffer_sprintf(wb, "\n[%s]\n", co->name);
 
-				pthread_rwlock_wrlock(&co->rwlock);
+				config_section_write_lock(co);
 				for(cv = co->values; cv ; cv = cv->next) {
 
 					if(used && !(cv->flags & CONFIG_VALUE_USED)) {
@@ -471,10 +521,9 @@ void generate_config(BUFFER *wb, int only_changed)
 					}
 					buffer_sprintf(wb, "\t%s%s = %s\n", ((!(cv->flags & CONFIG_VALUE_CHANGED)) && (cv->flags & CONFIG_VALUE_USED))?"# ":"", cv->name, cv->value);
 				}
-				pthread_rwlock_unlock(&co->rwlock);
+				config_section_unlock(co);
 			}
 		}
-		pthread_rwlock_unlock(&config_rwlock);
+		config_global_unlock();
 	}
 }
-
diff --git a/src/apps_plugin.c b/src/apps_plugin.c
index 3c4e3ee291..e1f03c0c87 100644
--- a/src/apps_plugin.c
+++ b/src/apps_plugin.c
@@ -39,6 +39,10 @@
 #include "procfile.h"
 #include "../config.h"
 
+#ifdef NETDATA_INTERNAL_CHECKS
+#include <sys/prctl.h>
+#endif
+
 #define MAX_COMPARE_NAME 100
 #define MAX_NAME 100
 #define MAX_CMDLINE 1024
@@ -2408,6 +2412,15 @@ int main(int argc, char **argv)
 	}
 	else info("Found NETDATA_CONFIG_DIR='%s'", config_dir);
 
+#ifdef NETDATA_INTERNAL_CHECKS
+	if(debug_flags != 0) {
+		struct rlimit rl = { RLIM_INFINITY, RLIM_INFINITY };
+		if(setrlimit(RLIMIT_CORE, &rl) != 0)
+			info("Cannot request unlimited core dumps for debugging... Proceeding anyway...");
+		prctl(PR_SET_DUMPABLE, 1, 0, 0, 0);
+	}
+#endif /* NETDATA_INTERNAL_CHECKS */
+
 	info("starting...");
 
 	procfile_adaptive_initial_allocation = 1;
diff --git a/src/avl.c b/src/avl.c
index c01b471d04..db19849453 100644
--- a/src/avl.c
+++ b/src/avl.c
@@ -316,7 +316,17 @@ void avl_init(avl_tree *t, int (*compar)(void *a, void *b)) {
 
 /* ------------------------------------------------------------------------- */
 
-void avl_lock(avl_tree_lock *t) {
+void avl_read_lock(avl_tree_lock *t) {
+#ifndef AVL_WITHOUT_PTHREADS
+#ifdef AVL_LOCK_WITH_MUTEX
+	pthread_mutex_lock(&t->mutex);
+#else
+	pthread_rwlock_rdlock(&t->rwlock);
+#endif
+#endif /* AVL_WITHOUT_PTHREADS */
+}
+
+void avl_write_lock(avl_tree_lock *t) {
 #ifndef AVL_WITHOUT_PTHREADS
 #ifdef AVL_LOCK_WITH_MUTEX
 	pthread_mutex_lock(&t->mutex);
@@ -357,29 +367,29 @@ void avl_init_lock(avl_tree_lock *t, int (*compar)(void *a, void *b)) {
 }
 
 int avl_range_lock(avl_tree_lock *t, avl *a, avl *b, int (*iter)(avl *), avl **ret) {
-	avl_lock(t);
+	avl_read_lock(t);
 	int ret2 = avl_range(&t->avl_tree, a, b, iter, ret);
 	avl_unlock(t);
 	return ret2;
 }
 
 int avl_removeroot_lock(avl_tree_lock *t) {
-	avl_lock(t);
+	avl_write_lock(t);
 	int ret = avl_removeroot(&t->avl_tree);
 	avl_unlock(t);
 	return ret;
 }
 
 int avl_remove_lock(avl_tree_lock *t, avl *a) {
-	avl_lock(t);
+	avl_write_lock(t);
 	int ret = avl_remove(&t->avl_tree, a);
 	avl_unlock(t);
 	return ret;
 }
 
 int avl_insert_lock(avl_tree_lock *t, avl *a) {
-	avl_lock(t);
+	avl_write_lock(t);
 	int ret = avl_insert(&t->avl_tree, a);
-	avl_lock(t);
+	avl_unlock(t);
 	return ret;
 }
diff --git a/src/main.c b/src/main.c
index 50c733d32f..5e9f497885 100644
--- a/src/main.c
+++ b/src/main.c
@@ -496,18 +496,17 @@ int main(int argc, char **argv)
 	// never become a problem
 	if(nice(20) == -1) error("Cannot lower my CPU priority.");
 
-	if(become_daemon(dont_fork, 0, user, input_log_file, output_log_file, error_log_file, access_log_file, &access_fd, &stdaccess) == -1) {
+	if(become_daemon(dont_fork, 0, user, input_log_file, output_log_file, error_log_file, access_log_file, &access_fd, &stdaccess) == -1)
 		fatal("Cannot demonize myself.");
-		exit(1);
-	}
 
+#ifdef NETDATA_INTERNAL_CHECKS
 	if(debug_flags != 0) {
 		struct rlimit rl = { RLIM_INFINITY, RLIM_INFINITY };
 		if(setrlimit(RLIMIT_CORE, &rl) != 0)
 			info("Cannot request unlimited core dumps for debugging... Proceeding anyway...");
-
 		prctl(PR_SET_DUMPABLE, 1, 0, 0, 0);
 	}
+#endif /* NETDATA_INTERNAL_CHECKS */
 
 	if(output_log_syslog || error_log_syslog || access_log_syslog)
 		openlog("netdata", LOG_PID, LOG_DAEMON);
@@ -515,7 +514,6 @@ int main(int argc, char **argv)
 	info("NetData started on pid %d", getpid());
 
 
-
 	// ------------------------------------------------------------------------
 	// get default pthread stack size