dsdb: Do not refresh the schema using the wrong event context

What we now do is have the refresh function and module be on a
seperate object to the schema, only referring to the data and
not excuting on the original ldb and event loop.

That is, we never use another ldb context when calling the
refresh function, by binding the refresh handler to the
ldb and not the schema.

Andrew Bartlett

Change-Id: I5c323dda743cf5858badd01147fda6227599bc16
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

5 files changed, 191 insertions, 112 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index fc725dfc6eb..d8bc8c7cf0d 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -39,7 +39,10 @@ struct schema_load_private_data {
 	struct tdb_wrap *metadata;
 };
 
-static int dsdb_schema_from_db(struct ldb_module *module, struct ldb_dn *schema_dn, uint64_t current_usn,
+static int dsdb_schema_from_db(struct ldb_module *module,
+			       TALLOC_CTX *mem_ctx,
+			       uint64_t current_usn,
+			       uint64_t schema_seq_num,
 			       struct dsdb_schema **schema);
 
 /*
@@ -155,9 +158,11 @@ static int schema_metadata_get_uint64(struct ldb_module *module,
 	return LDB_SUCCESS;
 }
 
-static struct dsdb_schema *dsdb_schema_refresh(struct ldb_module *module, struct dsdb_schema *schema, bool is_global_schema)
+static struct dsdb_schema *dsdb_schema_refresh(struct ldb_module *module, struct tevent_context *ev,
+					       struct dsdb_schema *schema, bool is_global_schema)
 {
-	uint64_t current_usn, value;
+	TALLOC_CTX *mem_ctx;
+	uint64_t current_usn, schema_seq_num = 0;
 	int ret;
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
 	struct dsdb_schema *new_schema;
@@ -175,11 +180,11 @@ static struct dsdb_schema *dsdb_schema_refresh(struct ldb_module *module, struct
 		return schema;
 	}
 
-	lastts = schema->last_refresh;
-	ts = time(NULL);
-	if (lastts > (ts - schema->refresh_interval)) {
-		DEBUG(11, ("Less than %d seconds since last reload, returning cached version ts = %d\n", (int)schema->refresh_interval, (int)lastts));
-		return schema;
+	SMB_ASSERT(ev == ldb_get_event_context(ldb));
+
+	mem_ctx = talloc_new(module);
+	if (mem_ctx == NULL) {
+		return NULL;
 	}
 
 	/*
@@ -190,29 +195,58 @@ static struct dsdb_schema *dsdb_schema_refresh(struct ldb_module *module, struct
 	 * continue to hit the database to get the highest USN.
 	 */
 
-	ret = schema_metadata_get_uint64(module, DSDB_METADATA_SCHEMA_SEQ_NUM, &value, 0);
-	if (ret == LDB_SUCCESS) {
-		schema->metadata_usn = value;
-	} else {
-		/* From an old provision it can happen that the tdb didn't exists yet */
-		DEBUG(0, ("Error while searching for the schema usn in the metadata\n"));
-		schema->metadata_usn = 0;
+	ret = schema_metadata_get_uint64(module, DSDB_METADATA_SCHEMA_SEQ_NUM, &schema_seq_num, 0);
+
+	if (schema != NULL) {
+		lastts = schema->last_refresh;
+		ts = time(NULL);
+		if (lastts > (ts - schema->refresh_interval)) {
+			DEBUG(11, ("Less than %d seconds since last reload, "
+				   "returning cached version ts = %d\n",
+				   (int)schema->refresh_interval,
+				   (int)lastts));
+			TALLOC_FREE(mem_ctx);
+			return schema;
+		}
+
+		if (ret == LDB_SUCCESS) {
+			schema->metadata_usn = schema_seq_num;
+		} else {
+			/* From an old provision it can happen that the tdb didn't exists yet */
+			DEBUG(0, ("Error while searching for the schema usn in the metadata\n"));
+			schema->metadata_usn = 0;
+		}
+		schema->last_refresh = ts;
+
 	}
-	schema->last_refresh = ts;
 
 	ret = dsdb_module_load_partition_usn(module, schema_dn, &current_usn, NULL, NULL);
-	if (ret != LDB_SUCCESS || current_usn == schema->loaded_usn) {
+	if (ret != LDB_SUCCESS || (schema && (current_usn == schema->loaded_usn))) {
+		TALLOC_FREE(mem_ctx);
 		return schema;
 	}
 
-	ret = dsdb_schema_from_db(module, schema_dn, current_usn, &new_schema);
+	ret = dsdb_schema_from_db(module, mem_ctx, current_usn, schema_seq_num, &new_schema);
 	if (ret != LDB_SUCCESS) {
+		ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+			      "dsdb_schema_from_db() failed: %d:%s: %s",
+			      ret, ldb_strerror(ret), ldb_errstring(ldb));
+		TALLOC_FREE(mem_ctx);
 		return schema;
 	}
 
+	ret = dsdb_set_schema(ldb, new_schema);
+	if (ret != LDB_SUCCESS) {
+		ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+			      "dsdb_set_schema() failed: %d:%s: %s",
+			      ret, ldb_strerror(ret), ldb_errstring(ldb));
+		TALLOC_FREE(mem_ctx);
+		return schema;
+	}
 	if (is_global_schema) {
 		dsdb_make_schema_global(ldb, new_schema);
 	}
+	TALLOC_FREE(mem_ctx);
 	return new_schema;
 }
 
@@ -221,13 +255,17 @@ static struct dsdb_schema *dsdb_schema_refresh(struct ldb_module *module, struct
   Given an LDB module (pointing at the schema DB), and the DN, set the populated schema
 */
 
-static int dsdb_schema_from_db(struct ldb_module *module, struct ldb_dn *schema_dn, uint64_t current_usn,
+static int dsdb_schema_from_db(struct ldb_module *module,
+			       TALLOC_CTX *mem_ctx,
+			       uint64_t current_usn,
+			       uint64_t schema_seq_num,
 			       struct dsdb_schema **schema)
 {
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
 	TALLOC_CTX *tmp_ctx;
 	char *error_string;
 	int ret;
+	struct ldb_dn *schema_dn = ldb_get_schema_basedn(ldb);
 	struct ldb_result *schema_res;
 	struct ldb_result *res;
 	static const char *schema_attrs[] = {
@@ -290,37 +328,11 @@ static int dsdb_schema_from_db(struct ldb_module *module, struct ldb_dn *schema_
 		goto failed;
 	}
 
-	(*schema)->refresh_in_progress = true;
-
-	/* If we have the readOnlySchema opaque, then don't check for
-	 * runtime schema updates, as they are not permitted (we would
-	 * have to update the backend server schema too */
-	if (!ldb_get_opaque(ldb, "readOnlySchema")) {
-		(*schema)->refresh_fn = dsdb_schema_refresh;
-		(*schema)->loaded_from_module = module;
-		(*schema)->loaded_usn = current_usn;
-	}
-
-	/* "dsdb_set_schema()" steals schema into the ldb_context */
-	ret = dsdb_set_schema(ldb, (*schema));
-
-	(*schema)->refresh_in_progress = false;
+	(*schema)->loaded_usn = current_usn;
+	(*schema)->metadata_usn = schema_seq_num;
 	(*schema)->last_refresh = time(NULL);
 
-	if (ret != LDB_SUCCESS) {
-		ldb_debug_set(ldb, LDB_DEBUG_FATAL,
-			      "schema_load_init: dsdb_set_schema() failed: %d:%s: %s",
-			      ret, ldb_strerror(ret), ldb_errstring(ldb));
-		goto failed;
-	}
-
-	/* Ensure this module won't go away before the callback.  This
-	 * causes every schema to have the LDB that originally loaded
-	 * the first schema as a talloc child. */
-	if (talloc_reference(*schema, ldb) == NULL) {
-		ldb_oom(ldb);
-		ret = LDB_ERR_OPERATIONS_ERROR;
-	}
+	talloc_steal(mem_ctx, *schema);
 
 failed:
 	if (flags & LDB_FLG_ENABLE_TRACING) {
@@ -335,11 +347,10 @@ failed:
 static int schema_load_init(struct ldb_module *module)
 {
 	struct schema_load_private_data *private_data;
-	struct dsdb_schema *schema;
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
+	struct dsdb_schema *schema;
+	void *readOnlySchema;
 	int ret;
-	uint64_t current_usn;
-	struct ldb_dn *schema_dn;
 
 	private_data = talloc_zero(module, struct schema_load_private_data);
 	if (private_data == NULL) {
@@ -353,39 +364,69 @@ static int schema_load_init(struct ldb_module *module)
 		return ret;
 	}
 
-	if (dsdb_get_schema(ldb, NULL)) {
-		return LDB_SUCCESS;
-	}
+	schema = dsdb_get_schema(ldb, NULL);
+
+	/* We might already have a schema */
+	if (schema != NULL) {
+		/* Hook up the refresh function */
+		if (dsdb_uses_global_schema(ldb)) {
+			ret = dsdb_set_schema_refresh_function(ldb, dsdb_schema_refresh, module);
+
+			if (ret != LDB_SUCCESS) {
+				ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+					      "schema_load_init: dsdb_set_schema_refresh_fns() failed: %d:%s: %s",
+					      ret, ldb_strerror(ret), ldb_errstring(ldb));
+				return ret;
+			}
+		}
 
-	schema_dn = ldb_get_schema_basedn(ldb);
-	if (!schema_dn) {
-		ldb_reset_err_string(ldb);
-		ldb_debug(ldb, LDB_DEBUG_WARNING,
-			  "schema_load_init: no schema dn present: (skip schema loading)\n");
 		return LDB_SUCCESS;
 	}
 
-	ret = dsdb_module_load_partition_usn(module, schema_dn, &current_usn, NULL, NULL);
-	if (ret != LDB_SUCCESS) {
-		/* Ignore the error and just reload the DB more often */
-		current_usn = 0;
-	}
+	readOnlySchema = ldb_get_opaque(ldb, "readOnlySchema");
 
-	ret = dsdb_schema_from_db(module, schema_dn, current_usn, &schema);
-	/* We don't care too much on the result of this action
-	 * the most probable reason for this to fail is that the tdb didn't
-	 * exists yet and this will be corrected by the partition module.
-	 */
-	if (ret == LDB_SUCCESS && schema_metadata_open(module) == LDB_SUCCESS) {
-		uint64_t value;
+	/* If we have the readOnlySchema opaque, then don't check for
+	 * runtime schema updates, as they are not permitted (we would
+	 * have to update the backend server schema too */
+	if (readOnlySchema != NULL) {
+		struct dsdb_schema *new_schema;
+		ret = dsdb_schema_from_db(module, private_data, 0, 0, &new_schema);
+		if (ret != LDB_SUCCESS) {
+			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+				      "schema_load_init: dsdb_schema_from_db() failed: %d:%s: %s",
+				      ret, ldb_strerror(ret), ldb_errstring(ldb));
+			return ret;
+		}
 
-		ret = schema_metadata_get_uint64(module, DSDB_METADATA_SCHEMA_SEQ_NUM, &value, 0);
-		if (ret == LDB_SUCCESS) {
-			schema->metadata_usn = value;
-		} else {
-			schema->metadata_usn = 0;
+		/* "dsdb_set_schema()" steals schema into the ldb_context */
+		ret = dsdb_set_schema(ldb, new_schema);
+		if (ret != LDB_SUCCESS) {
+			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+				      "schema_load_init: dsdb_set_schema() failed: %d:%s: %s",
+				      ret, ldb_strerror(ret), ldb_errstring(ldb));
+			return ret;
+		}
+
+	} else {
+		ret = dsdb_set_schema_refresh_function(ldb, dsdb_schema_refresh, module);
+
+		if (ret != LDB_SUCCESS) {
+			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+				      "schema_load_init: dsdb_set_schema_refresh_fns() failed: %d:%s: %s",
+				      ret, ldb_strerror(ret), ldb_errstring(ldb));
+			return ret;
 		}
 	}
+
+	schema = dsdb_get_schema(ldb, NULL);
+
+	/* We do this, invoking the refresh handler, so we know that it works */
+	if (schema == NULL) {
+		ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+			      "schema_load_init: dsdb_get_schema failed");
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
 	return ret;
 }
 
diff --git a/source4/dsdb/samdb/samdb.c b/source4/dsdb/samdb/samdb.c
index 361ece79f0a..4584a61d475 100644
--- a/source4/dsdb/samdb/samdb.c
+++ b/source4/dsdb/samdb/samdb.c
@@ -54,7 +54,6 @@ struct ldb_context *samdb_connect_url(TALLOC_CTX *mem_ctx,
 				  unsigned int flags, const char *url)
 {
 	struct ldb_context *ldb;
-	struct dsdb_schema *schema;
 	int ret;
 
 	ldb = ldb_wrap_find(url, ev_ctx, lp_ctx, session_info, NULL, flags);
@@ -74,12 +73,6 @@ struct ldb_context *samdb_connect_url(TALLOC_CTX *mem_ctx,
 		return NULL;
 	}
 
-	schema = dsdb_get_schema(ldb, NULL);
-	/* make the resulting schema global */
-	if (schema) {
-		dsdb_make_schema_global(ldb, schema);
-	}
-
 	if (!ldb_wrap_add(url, ev_ctx, lp_ctx, session_info, NULL, flags, ldb)) {
 		talloc_free(ldb);
 		return NULL;
diff --git a/source4/dsdb/schema/schema.h b/source4/dsdb/schema/schema.h
index 68c39c23927..cac6f98a27e 100644
--- a/source4/dsdb/schema/schema.h
+++ b/source4/dsdb/schema/schema.h
@@ -248,8 +248,6 @@ struct dsdb_schema {
 	} fsmo;
 
 	/* Was this schema loaded from ldb (if so, then we will reload it when we detect a change in ldb) */
-	struct ldb_module *loaded_from_module;
-	struct dsdb_schema *(*refresh_fn)(struct ldb_module *module, struct dsdb_schema *schema, bool is_global_schema);
 	bool refresh_in_progress;
 	time_t ts_last_change;
 	time_t last_refresh;
@@ -280,6 +278,11 @@ enum dsdb_schema_convert_target {
 	TARGET_AD_SCHEMA_SUBENTRY
 };
 
+struct ldb_module;
+
+typedef struct dsdb_schema *(*dsdb_schema_refresh_fn)(struct ldb_module *module,
+						      struct tevent_context *ev,
+						      struct dsdb_schema *schema, bool is_global_schema);
 #include "dsdb/schema/proto.h"
 
 #endif /* _DSDB_SCHEMA_H */
diff --git a/source4/dsdb/schema/schema_init.c b/source4/dsdb/schema/schema_init.c
index 43a6a89b3c3..d0388079241 100644
--- a/source4/dsdb/schema/schema_init.c
+++ b/source4/dsdb/schema/schema_init.c
@@ -97,8 +97,6 @@ struct dsdb_schema *dsdb_schema_copy_shallow(TALLOC_CTX *mem_ctx,
 	}
 
 	/* leave reload_seq_number = 0 so it will be refresh ASAP */
-	schema_copy->refresh_fn = schema->refresh_fn;
-	schema_copy->loaded_from_module = schema->loaded_from_module;
 
 	return schema_copy;
 
diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index 78c6e56bb50..6029e46a7a1 100644
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -443,6 +443,26 @@ failed:
  * Attach the schema to an opaque pointer on the ldb,
  * so ldb modules can find it
  */
+int dsdb_set_schema_refresh_function(struct ldb_context *ldb,
+				     dsdb_schema_refresh_fn refresh_fn,
+				     struct ldb_module *module)
+{
+	int ret = ldb_set_opaque(ldb, "dsdb_schema_refresh_fn", refresh_fn);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	ret = ldb_set_opaque(ldb, "dsdb_schema_refresh_fn_private_data", module);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+	return LDB_SUCCESS;
+}
+
+/**
+ * Attach the schema to an opaque pointer on the ldb,
+ * so ldb modules can find it
+ */
 int dsdb_set_schema(struct ldb_context *ldb, struct dsdb_schema *schema)
 {
 	struct dsdb_schema *old_schema;
@@ -467,6 +487,8 @@ int dsdb_set_schema(struct ldb_context *ldb, struct dsdb_schema *schema)
 		talloc_steal(ldb, schema);
 	}
 
+	talloc_steal(ldb, schema);
+
 	ret = ldb_set_opaque(ldb, "dsdb_use_global_schema", NULL);
 	if (ret != LDB_SUCCESS) {
 		return ret;
@@ -518,6 +540,16 @@ int dsdb_reference_schema(struct ldb_context *ldb, struct dsdb_schema *schema,
 		return ret;
 	}
 
+	ret = ldb_set_opaque(ldb, "dsdb_refresh_fn", NULL);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	ret = ldb_set_opaque(ldb, "dsdb_refresh_fn_private_data", NULL);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
 	ret = dsdb_schema_set_indices_and_attributes(ldb, schema, write_indices_and_attributes);
 	if (ret != LDB_SUCCESS) {
 		return ret;
@@ -533,14 +565,15 @@ int dsdb_set_global_schema(struct ldb_context *ldb)
 {
 	int ret;
 	void *use_global_schema = (void *)1;
-	if (!global_schema) {
-		return LDB_SUCCESS;
-	}
 	ret = ldb_set_opaque(ldb, "dsdb_use_global_schema", use_global_schema);
 	if (ret != LDB_SUCCESS) {
 		return ret;
 	}
 
+	if (global_schema == NULL) {
+		return LDB_SUCCESS;
+	}
+
 	/* Set the new attributes based on the new schema */
 	ret = dsdb_schema_set_indices_and_attributes(ldb, global_schema, false /* Don't write indices and attributes, it's expensive */);
 	if (ret == LDB_SUCCESS) {
@@ -567,11 +600,13 @@ bool dsdb_uses_global_schema(struct ldb_context *ldb)
 struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *reference_ctx)
 {
 	const void *p;
-	struct dsdb_schema *schema_out;
-	struct dsdb_schema *schema_in;
+	struct dsdb_schema *schema_out = NULL;
+	struct dsdb_schema *schema_in = NULL;
+	dsdb_schema_refresh_fn refresh_fn;
+	struct ldb_module *loaded_from_module;
 	bool use_global_schema;
 	TALLOC_CTX *tmp_ctx = talloc_new(reference_ctx);
-	if (!tmp_ctx) {
+	if (tmp_ctx == NULL) {
 		return NULL;
 	}
 
@@ -581,29 +616,38 @@ struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *referen
 		schema_in = global_schema;
 	} else {
 		p = ldb_get_opaque(ldb, "dsdb_schema");
-
-		schema_in = talloc_get_type(p, struct dsdb_schema);
-		if (!schema_in) {
-			talloc_free(tmp_ctx);
-			return NULL;
+		if (p != NULL) {
+			schema_in = talloc_get_type_abort(p, struct dsdb_schema);
 		}
 	}
 
-	if (schema_in->refresh_fn && !schema_in->refresh_in_progress) {
-		if (!talloc_reference(tmp_ctx, schema_in)) {
-			/*
-			 * ensure that the schema_in->refresh_in_progress
-			 * remains valid for the right amount of time
-			 */
-			talloc_free(tmp_ctx);
-			return NULL;
+	refresh_fn = ldb_get_opaque(ldb, "dsdb_schema_refresh_fn");
+	if (refresh_fn) {
+		loaded_from_module = ldb_get_opaque(ldb, "dsdb_schema_refresh_fn_private_data");
+
+		SMB_ASSERT(loaded_from_module && (ldb_module_get_ctx(loaded_from_module) == ldb));
+	}
+
+	if (refresh_fn) {
+		/* We need to guard against recurisve calls here */
+		if (ldb_set_opaque(ldb, "dsdb_schema_refresh_fn", NULL) != LDB_SUCCESS) {
+			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+				      "dsdb_get_schema: clearing dsdb_schema_refresh_fn failed");
+		} else {
+			schema_out = refresh_fn(loaded_from_module,
+						ldb_get_event_context(ldb),
+						schema_in,
+						use_global_schema);
+		}
+		if (ldb_set_opaque(ldb, "dsdb_schema_refresh_fn", refresh_fn) != LDB_SUCCESS) {
+			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+				      "dsdb_get_schema: re-setting dsdb_schema_refresh_fn failed");
+		}
+		if (!schema_out) {
+			schema_out = schema_in;
+			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
+				      "dsdb_get_schema: refresh_fn() failed");
 		}
-		schema_in->refresh_in_progress = true;
-		/* This may change schema, if it needs to reload it from disk */
-		schema_out = schema_in->refresh_fn(schema_in->loaded_from_module,
-						   schema_in,
-						   use_global_schema);
-		schema_in->refresh_in_progress = false;
 	} else {
 		schema_out = schema_in;
 	}