ldb:tests: Add test to show that locks are released on TALLOC_FREE(req)

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 68b7c79fb6582e5a9f21ace4a210f8879e72e64b..96d2dd27df25ea7d2851aed0dbd0dcb8b1f0b784 100644 (file)
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -2076,6 +2076,229 @@ static void test_ldb_modify_during_whole_search(void **state)
        assert_int_equal(res2->count, 1);
 }
 
+/*
+ * This test is also complex.
+ *
+ * The purpose is to test if a modify can occur during an ldb_search()
+ * before the request is destroyed with TALLOC_FREE()
+ *
+ * This would be a failure if in process
+ * (1) and (2):
+ *  - (1) ldb_search() starts and waits
+ *  - (2) an entry in the DB is allowed to change before the ldb_wait() is called
+ *  - (1) the original process can see the modification before the TALLOC_FREE()
+ * also we check that
+ *  - (1) the original process can see the modification after the TALLOC_FREE()
+ *
+ */
+
+/*
+ * This purpose of this callback is to trigger a write in
+ * the child process before the ldb_wait() is called
+ *
+ * In ldb 1.1.31 ldb_search() omitted to take a all-record
+ * lock for the full duration of the search and callbacks
+ *
+ * We assume that if the write will proceed, it will proceed in a 3
+ * second window after the function is called.
+ */
+
+static int test_ldb_modify_before_ldb_wait_callback1(struct ldb_request *req,
+                                                    struct ldb_reply *ares)
+{
+       switch (ares->type) {
+       case LDB_REPLY_ENTRY:
+       case LDB_REPLY_REFERRAL:
+               return LDB_SUCCESS;
+
+       case LDB_REPLY_DONE:
+               break;
+       }
+
+       return ldb_request_done(req, LDB_SUCCESS);
+}
+
+static void test_ldb_modify_before_ldb_wait(void **state)
+{
+       struct search_test_ctx *search_test_ctx = talloc_get_type_abort(*state,
+                       struct search_test_ctx);
+       int ret;
+       struct ldb_request *req;
+       pid_t pid;
+       int wstatus;
+       struct ldb_dn *search_dn;
+       struct ldb_dn *basedn;
+       struct ldb_result *res2;
+       int pipes[2];
+       char buf[2];
+       pid_t child_pid;
+       unsigned res_count;
+
+       search_dn = ldb_dn_new_fmt(search_test_ctx,
+                                  search_test_ctx->ldb_test_ctx->ldb,
+                                  "cn=test_search_cn,"
+                                  "dc=search_test_entry");
+       assert_non_null(search_dn);
+
+       basedn = ldb_dn_new_fmt(search_test_ctx,
+                               search_test_ctx->ldb_test_ctx->ldb,
+                               "%s",
+                               search_test_ctx->base_dn);
+       assert_non_null(basedn);
+
+       /*
+        * The search just needs to call DONE, we don't care about the
+        * contents of the search for this test
+        */
+       ret = ldb_build_search_req(&req,
+                                  search_test_ctx->ldb_test_ctx->ldb,
+                                  search_test_ctx,
+                                  basedn,
+                                  LDB_SCOPE_SUBTREE,
+                                  "(&(!(filterAttr=*))"
+                                  "(cn=test_search_cn))",
+                                  NULL,
+                                  NULL,
+                                  NULL,
+                                  test_ldb_modify_before_ldb_wait_callback1,
+                                  NULL);
+       assert_int_equal(ret, 0);
+       ret = ldb_request(search_test_ctx->ldb_test_ctx->ldb, req);
+
+       ret = pipe(pipes);
+       assert_int_equal(ret, 0);
+
+       child_pid = fork();
+       if (child_pid == 0) {
+               TALLOC_CTX *tmp_ctx = NULL;
+               struct ldb_message *msg;
+               struct ldb_message_element *el;
+               TALLOC_FREE(search_test_ctx->ldb_test_ctx->ldb);
+               TALLOC_FREE(search_test_ctx->ldb_test_ctx->ev);
+               search_test_ctx->ldb_test_ctx->ev = tevent_context_init(search_test_ctx->ldb_test_ctx);
+               if (search_test_ctx->ldb_test_ctx->ev == NULL) {
+                       exit(LDB_ERR_OPERATIONS_ERROR);
+               }
+
+               search_test_ctx->ldb_test_ctx->ldb = ldb_init(search_test_ctx->ldb_test_ctx,
+                                            search_test_ctx->ldb_test_ctx->ev);
+               if (search_test_ctx->ldb_test_ctx->ldb == NULL) {
+                       exit(LDB_ERR_OPERATIONS_ERROR);
+               }
+
+               ret = ldb_connect(search_test_ctx->ldb_test_ctx->ldb,
+                                 search_test_ctx->ldb_test_ctx->dbpath, 0, NULL);
+               if (ret != LDB_SUCCESS) {
+                       exit(ret);
+               }
+
+               tmp_ctx = talloc_new(search_test_ctx->ldb_test_ctx);
+               if (tmp_ctx == NULL) {
+                       exit(LDB_ERR_OPERATIONS_ERROR);
+               }
+
+               msg = ldb_msg_new(tmp_ctx);
+               if (msg == NULL) {
+                       exit(LDB_ERR_OPERATIONS_ERROR);
+               }
+
+               /*
+                * We must re-create this DN from a string to ensure
+                * it does not reference the now-gone LDB context of
+                * the parent
+                */
+               msg->dn = ldb_dn_new_fmt(search_test_ctx,
+                                        search_test_ctx->ldb_test_ctx->ldb,
+                                        "cn=test_search_cn,"
+                                        "dc=search_test_entry");
+
+               if (msg->dn == NULL) {
+                       exit(LDB_ERR_OPERATIONS_ERROR);
+               }
+
+               ret = ldb_msg_add_string(msg, "filterAttr", "TRUE");
+               if (ret != 0) {
+                       exit(LDB_ERR_OPERATIONS_ERROR);
+               }
+               el = ldb_msg_find_element(msg, "filterAttr");
+               if (el == NULL) {
+                       exit(LDB_ERR_OPERATIONS_ERROR);
+               }
+               el->flags = LDB_FLAG_MOD_REPLACE;
+
+               ret = ldb_transaction_start(search_test_ctx->ldb_test_ctx->ldb);
+               if (ret != 0) {
+                       exit(ret);
+               }
+
+               if (write(pipes[1], "GO", 2) != 2) {
+                       exit(LDB_ERR_OPERATIONS_ERROR);
+               }
+
+               ret = ldb_modify(search_test_ctx->ldb_test_ctx->ldb, msg);
+               if (ret != 0) {
+                       exit(ret);
+               }
+
+               ret = ldb_transaction_commit(search_test_ctx->ldb_test_ctx->ldb);
+               exit(ret);
+       }
+
+       ret = read(pipes[0], buf, 2);
+       assert_int_equal(ret, 2);
+
+       sleep(3);
+
+       /*
+        * If writes are not blocked until after the (never called) ldb_wait(), we
+        * will be able to successfully search for this modification
+        * here
+        */
+
+       ret = ldb_search(search_test_ctx->ldb_test_ctx->ldb, search_test_ctx,
+                        &res2, search_dn, LDB_SCOPE_BASE, NULL,
+                        "filterAttr=TRUE");
+
+       /*
+        * We avoid making assertions before TALLOC_FREE()ing the request,
+        * lest the assert fail and mess with the clean-up because we still
+        * have locks.
+        */
+       res_count = res2->count;
+       TALLOC_FREE(req);
+
+       /* We should not have got the result */
+       assert_int_equal(res_count, 0);
+       assert_int_equal(ret, 0);
+
+       pid = waitpid(child_pid, &wstatus, 0);
+       assert_int_equal(pid, child_pid);
+
+       assert_true(WIFEXITED(wstatus));
+
+       assert_int_equal(WEXITSTATUS(wstatus), 0);
+
+       /*
+        * If writes are blocked until after the search request was freed, we
+        * will be able to successfully search for this modification
+        * now
+        */
+
+       search_dn = ldb_dn_new_fmt(search_test_ctx,
+                                  search_test_ctx->ldb_test_ctx->ldb,
+                                  "cn=test_search_cn,"
+                                  "dc=search_test_entry");
+
+       ret = ldb_search(search_test_ctx->ldb_test_ctx->ldb,
+                        search_test_ctx,
+                        &res2, search_dn, LDB_SCOPE_BASE, NULL,
+                        "filterAttr=TRUE");
+       assert_int_equal(ret, 0);
+
+       /* We got the result */
+       assert_int_equal(res2->count, 1);
+}
+
 static int ldb_case_test_setup(void **state)
 {
        int ret;
@@ -2640,6 +2863,9 @@ int main(int argc, const char **argv)
                cmocka_unit_test_setup_teardown(test_ldb_modify_during_whole_search,
                                                ldb_search_test_setup,
                                                ldb_search_test_teardown),
+               cmocka_unit_test_setup_teardown(test_ldb_modify_before_ldb_wait,
+                                               ldb_search_test_setup,
+                                               ldb_search_test_teardown),
                cmocka_unit_test_setup_teardown(test_ldb_attrs_case_insensitive,
                                                ldb_case_test_setup,
                                                ldb_case_test_teardown),