[asterisk-commits] twilson: branch twilson/res_config_sqlite3 r334353 - /team/twilson/res_config...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Sep 2 14:50:23 CDT 2011


Author: twilson
Date: Fri Sep  2 14:50:13 2011
New Revision: 334353

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=334353
Log:
Fix reload, batch, and refcount issues

Modified:
    team/twilson/res_config_sqlite3/res/res_config_sqlite3.c

Modified: team/twilson/res_config_sqlite3/res/res_config_sqlite3.c
URL: http://svnview.digium.com/svn/asterisk/team/twilson/res_config_sqlite3/res/res_config_sqlite3.c?view=diff&rev=334353&r1=334352&r2=334353
==============================================================================
--- team/twilson/res_config_sqlite3/res/res_config_sqlite3.c (original)
+++ team/twilson/res_config_sqlite3/res/res_config_sqlite3.c Fri Sep  2 14:50:13 2011
@@ -91,6 +91,7 @@
 	unsigned int dirty:1;
 	unsigned int debug:1;
 	unsigned int exiting:1;
+	unsigned int wakeup:1;
 	unsigned int batch;
 };
 
@@ -100,6 +101,8 @@
 AST_MUTEX_DEFINE_STATIC(config_lock);
 
 static int realtime_sqlite3_execute_handle(struct realtime_sqlite3_db *db, const char *sql, int (*callback)(void*, int, char **, char **), void *arg, int sync);
+void db_start_batch(struct realtime_sqlite3_db *db);
+void db_stop_batch(struct realtime_sqlite3_db *db);
 
 static int db_hash_fn(const void *obj, const int flags)
 {
@@ -121,8 +124,7 @@
 
 	ast_debug(1, "Destroying db: %s\n", db->name);
 	ast_string_field_free_memory(db);
-	db->exiting = 1;
-	ast_cond_signal(&db->cond);
+	db_stop_batch(db);
 	if (db->handle) {
 		ao2_lock(db);
 		sqlite3_close(db->handle);
@@ -156,7 +158,11 @@
 static int is_dirty_cb(void *obj, void *arg, int flags)
 {
 	struct realtime_sqlite3_db *db = obj;
-	return db->dirty ? CMP_MATCH : 0;
+	if (db->dirty) {
+		db_stop_batch(db);
+		return CMP_MATCH;
+	}
+	return 0;
 }
 
 static void unlink_dirty_databases(void)
@@ -188,8 +194,10 @@
 	ao2_lock(db);
 	realtime_sqlite3_execute_handle(db, "BEGIN TRANSACTION", NULL, NULL, 0);
 	for (;;) {
-		/* We're ok with spurious wakeups, so we don't worry about a predicate */
-		ast_cond_wait(&db->cond, ao2_object_get_lockaddr(db));
+		if (!db->wakeup) {
+			ast_cond_wait(&db->cond, ao2_object_get_lockaddr(db));
+		}
+		db->wakeup = 0;
 		if (realtime_sqlite3_execute_handle(db, "COMMIT", NULL, NULL, 0) < 0) {
 			realtime_sqlite3_execute_handle(db, "ROLLBACK", NULL, NULL, 0);
 		}
@@ -208,47 +216,71 @@
 	return NULL;
 }
 
-static int create_or_update_db(struct ast_config *config, const char *cat)
+static int db_open(struct realtime_sqlite3_db *db)
+{
+	ao2_lock(db);
+	if (sqlite3_open(db->filename, &db->handle) != SQLITE_OK) {
+		ast_log(LOG_WARNING, "Could not open %s: %s\n", db->filename, sqlite3_errmsg(db->handle));
+		ao2_unlock(db);
+		return -1;
+	}
+
+	if (db->debug) {
+		sqlite3_trace(db->handle, trace_cb, db);
+	} else {
+		sqlite3_trace(db->handle, NULL, NULL);
+	}
+
+	ao2_unlock(db);
+
+	return 0;
+}
+
+static void db_sync(struct realtime_sqlite3_db *db)
+{
+	db->wakeup = 1;
+	ast_cond_signal(&db->cond);
+}
+
+void db_start_batch(struct realtime_sqlite3_db *db)
+{
+	if (db->batch) {
+		ast_cond_init(&db->cond, NULL);
+		ao2_ref(db, +1);
+		ast_pthread_create_background(&db->syncthread, NULL, db_sync_thread, db);
+	}
+}
+
+void db_stop_batch(struct realtime_sqlite3_db *db)
+{
+	if (db->batch) {
+		db->exiting = 1;
+		db_sync(db);
+		pthread_join(db->syncthread, NULL);
+	}
+}
+
+static struct realtime_sqlite3_db *new_realtime_sqlite3_db(struct ast_config *config, const char *cat)
 {
 	struct ast_variable *var;
 	struct realtime_sqlite3_db *db;
-	int new = 0, reopen = 0;
-
-	if (!(db = find_database(cat))) {
-		if (!(db = ao2_alloc(sizeof(*db), db_destructor))) {
-			return -1;
-		}
-		new = 1;
-	}
-
-	/* At this point db has a refcount of 1 (alloc'd) if new or 2 (linked and found) if !new
-	 * It is very important that any error exit unlinks !new entries. We could just throw
-	 * away the find_database ref above and rely on the link, but we can't be guaranteed
-	 * that some code sometime won't be added that unlinks us from another thread, etc. */
-
-	ao2_lock(db);
-
-	if (new && ast_string_field_init(db, 64)) {
-		ao2_unlock(db);
+
+	if (!(db = ao2_alloc(sizeof(*db), db_destructor))) {
+		return NULL;
+	}
+
+	if (ast_string_field_init(db, 64)) {
 		unref_db(&db);
-		return -1;
+		return NULL;
 	}
 
 	/* Set defaults */
 	db->requirements = REALTIME_SQLITE3_REQ_WARN;
 	db->batch = 100;
-	db->debug = 0;
-	db->dirty = 0;
 	ast_string_field_set(db, name, cat);
 
 	for (var = ast_variable_browse(config, cat); var; var = var->next) {
 		if (!strcasecmp(var->name, "dbfile")) {
-			if (!new && strcmp(db->filename, var->value)) {
-				/* We are updating the db with this name to a new location */
-				ast_debug(1, "DB %s moving from %s to %s\n", db->name, db->filename, var->value);
-				sqlite3_close(db->handle);
-				reopen = 1;
-			}
 			ast_string_field_set(db, filename, var->value);
 		} else if (!strcasecmp(var->name, "requirements")) {
 			db->requirements = str_to_requirements(var->value);
@@ -261,37 +293,53 @@
 
 	if (ast_strlen_zero(db->filename)) {
 		ast_log(LOG_WARNING, "Must specify dbfile in res_config_sqlite3.conf\n");
-		ao2_unlock(db);
 		unref_db(&db);
-		if (!new) {
-			ao2_unlink(databases, db);
-		}
-		return -1;
-	}
-
-	if ((new || reopen) && sqlite3_open(db->filename, &db->handle) != SQLITE_OK) {
-		ast_log(LOG_WARNING, "Could not open %s: %s\n", db->filename, sqlite3_errmsg(db->handle));
-		ao2_unlock(db);
-		unref_db(&db);
-		if (!new) {
-			ao2_unlink(databases, db);
-		}
-		return -1;
-	}
-
-	if (db->debug) {
-		sqlite3_trace(db->handle, trace_cb, db);
-	}
-
-	if (new) {
-		ao2_link(databases, db);
-		ast_cond_init(&db->cond, NULL);
-		ao2_ref(db, +1);
-		ast_pthread_create_background(&db->syncthread, NULL, db_sync_thread, db);
-	}
-
-	ao2_unlock(db);
-	unref_db(&db);
+		return NULL;
+	}
+
+	return db;
+}
+
+static int update_realtime_sqlite3_db(struct realtime_sqlite3_db *db, struct ast_config *config, const char *cat)
+{
+	struct realtime_sqlite3_db *new;
+
+	if (!(new = new_realtime_sqlite3_db(config, cat))) {
+		return -1;
+	}
+
+	/* Copy fields that don't need anything special done on change */
+	db->requirements = new->requirements;
+
+	/* Handle changes that require immediate behavior modification */
+	if (db->debug != new->debug) {
+		if (db->debug) {
+			sqlite3_trace(db->handle, NULL, NULL);
+		} else {
+			sqlite3_trace(db->handle, trace_cb, db);
+		}
+		db->debug = new->debug;
+	}
+
+	if (strcmp(db->filename, new->filename)) {
+		sqlite3_close(db->handle);
+		ast_string_field_set(db, filename, new->filename);
+		db_open(db); /* Also handles setting appropriate debug on new handle */
+	}
+
+	if (db->batch != new->batch) {
+		if (db->batch == 0) {
+			db->batch = new->batch;
+			db_start_batch(db);
+		} else if (new->batch == 0) {
+			db->batch = new->batch;
+			db_stop_batch(db);
+		}
+		db->batch = new->batch;
+	}
+
+	db->dirty = 0;
+	unref_db(&new);
 
 	return 0;
 }
@@ -421,7 +469,7 @@
 	ao2_unlock(db);
 
 	if (sync) {
-		ast_cond_signal(&db->cond);
+		db_sync(db);
 	}
 
 	return res;
@@ -912,10 +960,10 @@
 	return -1;
 }
 
-static int parse_config(void)
+static int parse_config(int reload)
 {
 	struct ast_config *config;
-	struct ast_flags config_flags = { CONFIG_FLAG_NOREALTIME | CONFIG_FLAG_FILEUNCHANGED };
+	struct ast_flags config_flags = { CONFIG_FLAG_NOREALTIME | (reload ? CONFIG_FLAG_FILEUNCHANGED : 0) };
 	static const char *config_filename = "res_config_sqlite3.conf";
 
 	config = ast_config_load(config_filename, config_flags);
@@ -932,14 +980,31 @@
 			config == CONFIG_STATUS_FILEMISSING ? "Missing" : "Invalid", config_filename);
 	} else {
 		const char *cat;
+		struct realtime_sqlite3_db *db;
 
 		mark_all_databases_dirty();
 		for (cat = ast_category_browse(config, NULL); cat; cat = ast_category_browse(config, cat)) {
 			if (!strcasecmp(cat, "general")) {
 				continue;
 			}
-			if (create_or_update_db(config, cat)) {
-				continue;
+			if (!(db = find_database(cat))) {
+				if (!(db = new_realtime_sqlite3_db(config, cat))) {
+					ast_log(LOG_WARNING, "Could not allocate new db for '%s' - skipping.\n", cat);
+					continue;
+				}
+				if (db_open(db)) {
+					unref_db(&db);
+					continue;
+				}
+				db_start_batch(db);
+				ao2_link(databases, db);
+				unref_db(&db);
+			} else  {
+				if (update_realtime_sqlite3_db(db, config, cat)) {
+					unref_db(&db); 
+					continue;
+				}
+				unref_db(&db);
 			}
 		}
 		unlink_dirty_databases();
@@ -954,17 +1019,30 @@
 
 static int reload(void)
 {
-	parse_config();
-	return 0;
+	parse_config(1);
+	return 0;
+}
+
+static int stop_batch_cb(void *obj, void *arg, int flags)
+{
+	struct realtime_sqlite3_db *db = obj;
+
+	db_stop_batch(db);
+	return CMP_MATCH;
 }
 
 static int unload_module(void)
 {
 	ast_mutex_lock(&config_lock);
+	/* Go ahead and manually unlink everything so that the the batch threads can
+	 * release their refs */
+	ao2_callback(databases, OBJ_MULTIPLE | OBJ_NODATA | OBJ_UNLINK, stop_batch_cb, NULL);
 	ao2_ref(databases, -1);
 	databases = NULL;
 	ast_mutex_unlock(&config_lock);
 
+	ast_config_engine_deregister(&sqlite3_config_engine);
+
 	return 0;
 }
 
@@ -974,7 +1052,7 @@
 		return AST_MODULE_LOAD_FAILURE;
 	}
 
-	if (parse_config()) {
+	if (parse_config(0)) {
 		ao2_ref(databases, -1);
 		return AST_MODULE_LOAD_FAILURE;
 	}




More information about the asterisk-commits mailing list