s4:dsdb Ensure we free old schema copies

It was reported by aatanasov that we kept around one whole schema per
modification made.  This does not fix that, but I hope moves us closer
to a fix

The most important part of the fix is that:

-		if (schema_out != schema_in) {
-			talloc_unlink(schema_in, ldb);
-		}

was the wrong way around.  This is now handled in the schema_set calls.

Andrew Bartlett

diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index b5d8ae47f5b3e3bad29fa8a5339b564eed030386..da5ad3ce9248e50737835dc8afbee922bc73ae2b 100644 (file)
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -366,6 +366,7 @@ int dsdb_setup_schema_inversion(struct ldb_context *ldb, struct dsdb_schema *sch
 
 int dsdb_set_schema(struct ldb_context *ldb, struct dsdb_schema *schema)
 {
+       struct dsdb_schema *old_schema;
        int ret;
 
        ret = dsdb_setup_sorted_accessors(ldb, schema);
@@ -378,10 +379,17 @@ int dsdb_set_schema(struct ldb_context *ldb, struct dsdb_schema *schema)
                return ret;
        }
 
+       old_schema = ldb_get_opaque(ldb, "dsdb_schema");
+
        ret = ldb_set_opaque(ldb, "dsdb_schema", schema);
        if (ret != LDB_SUCCESS) {
                return ret;
        }
+       /* Remove the refernece to the schema we just overwrote - if there was none, NULL is harmless here */
+       if (old_schema != schema) {
+               talloc_unlink(ldb, old_schema);
+               talloc_steal(ldb, schema);
+       }
 
        ret = ldb_set_opaque(ldb, "dsdb_use_global_schema", NULL);
        if (ret != LDB_SUCCESS) {
@@ -394,8 +402,6 @@ int dsdb_set_schema(struct ldb_context *ldb, struct dsdb_schema *schema)
                return ret;
        }
 
-       talloc_steal(ldb, schema);
-
        return LDB_SUCCESS;
 }
 
@@ -411,11 +417,16 @@ int dsdb_reference_schema(struct ldb_context *ldb, struct dsdb_schema *schema,
                          bool write_attributes)
 {
        int ret;
+       struct dsdb_schema *old_schema;
+       old_schema = ldb_get_opaque(ldb, "dsdb_schema");
        ret = ldb_set_opaque(ldb, "dsdb_schema", schema);
        if (ret != LDB_SUCCESS) {
                return ret;
        }
 
+       /* Remove the refernece to the schema we just overwrote - if there was none, NULL is harmless here */
+       talloc_unlink(ldb, old_schema);
+
        if (talloc_reference(ldb, schema) == NULL) {
                return LDB_ERR_OPERATIONS_ERROR;
        }
@@ -438,7 +449,6 @@ int dsdb_set_global_schema(struct ldb_context *ldb)
        if (!global_schema) {
                return LDB_SUCCESS;
        }
-
        ret = ldb_set_opaque(ldb, "dsdb_use_global_schema", use_global_schema);
        if (ret != LDB_SUCCESS) {
                return ret;
@@ -468,6 +478,10 @@ struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *referen
        struct dsdb_schema *schema_out;
        struct dsdb_schema *schema_in;
        bool use_global_schema;
+       TALLOC_CTX *tmp_ctx = talloc_new(reference_ctx);
+       if (!tmp_ctx) {
+               return NULL;
+       }
 
        /* see if we have a cached copy */
        use_global_schema = (ldb_get_opaque(ldb, "dsdb_use_global_schema") != NULL);
@@ -478,24 +492,29 @@ struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *referen
 
                schema_in = talloc_get_type(p, struct dsdb_schema);
                if (!schema_in) {
+                       talloc_free(tmp_ctx);
                        return NULL;
                }
        }
 
        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;
+               }
                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;
-               if (schema_out != schema_in) {
-                       talloc_unlink(schema_in, ldb);
-               }
        } else {
                schema_out = schema_in;
        }
 
+       /* This removes the extra reference above */
+       talloc_free(tmp_ctx);
        if (!reference_ctx) {
                return schema_out;
        } else {