The branch, master has been updated via 9703464 pidl: Fix Coverity warnings from duplicate NULL checks. via 47a8fb8 ldb: Do not use mktemp() nor leak files into /tmp during api.py test via be87e9d ldb: Add test for transaction deadlock detected when waiting for a search via e7f3b45 ldb: Add some tests to clarify the current iterator behaviour from 0b1ba00 testprogs: Ignore escape characters when printing test name
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 9703464b942fdddbf7bc4380cbd26d1803f9bc00 Author: Jeremy Allison <j...@samba.org> Date: Tue May 2 08:10:40 2017 -0700 pidl: Fix Coverity warnings from duplicate NULL checks. Pair-Programmed-With: Stefan Metzmacher <me...@samba.org> Signed-off-by: Jeremy Allison <j...@samba.org> Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Sat May 6 16:03:17 CEST 2017 on sn-devel-144 commit 47a8fb8e70feb631ddd02fa102467ce676456fa7 Author: Andrew Bartlett <abart...@samba.org> Date: Tue Apr 25 20:14:33 2017 +1200 ldb: Do not use mktemp() nor leak files into /tmp during api.py test Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Garming Sam <garm...@catalyst.net.nz> commit be87e9dfa39cc64f28e546a0999bf14e6146d289 Author: Andrew Bartlett <abart...@samba.org> Date: Fri Apr 7 11:38:11 2017 +1200 ldb: Add test for transaction deadlock detected when waiting for a search This was the original intent of 7dd31288a701d772e45b1960ac4ce4cc1be782ed but was broken in 251aaafe3a9213118ac3a92def9ab2104c40d12a and hidden by 4bb2958f16cc6af43d113528407d53f0d78b0486. Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Garming Sam <garm...@catalyst.net.nz> commit e7f3b457d6cbf6849e4f74a8524940fd61af316b Author: Andrew Bartlett <abart...@samba.org> Date: Fri Apr 7 09:32:05 2017 +1200 ldb: Add some tests to clarify the current iterator behaviour search_iterator() is no more memory efficient than search() because all the results come back at the first res.next() call Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Garming Sam <garm...@catalyst.net.nz> ----------------------------------------------------------------------- Summary of changes: lib/ldb/tests/ldb_mod_op_test.c | 243 ++++++++++++++++++++++++++++++ lib/ldb/tests/python/api.py | 276 ++++++++++++++++++++++++++--------- pidl/lib/Parse/Pidl/Samba4/Python.pm | 63 ++++---- 3 files changed, 484 insertions(+), 98 deletions(-) Changeset truncated at 500 lines: diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c index e609d3a..84043cb 100644 --- a/lib/ldb/tests/ldb_mod_op_test.c +++ b/lib/ldb/tests/ldb_mod_op_test.c @@ -21,10 +21,17 @@ #include <errno.h> #include <unistd.h> #include <talloc.h> + +#define TEVENT_DEPRECATED 1 +#include <tevent.h> + #include <ldb.h> #include <string.h> #include <ctype.h> +#include <sys/wait.h> + + #define DEFAULT_BE "tdb" #ifndef TEST_BE @@ -1165,6 +1172,239 @@ static void test_search_match_basedn(void **state) assert_int_equal(ret, 0); } + +/* + * This test is complex. + * The purpose is to test for a deadlock detected between ldb_search() + * and ldb_transaction_commit(). The deadlock happens if in process + * (1) and (2): + * - (1) the all-record lock is taken in ltdb_search() + * - (2) the ldb_transaction_start() call is made + * - (1) an un-indexed search starts (forced here by doing it in + * the callback + * - (2) the ldb_transaction_commit() is called. + * This returns LDB_ERR_BUSY if the deadlock is detected + * + * With ldb 1.1.29 and tdb 1.3.12 we avoid this only due to a missing + * lock call in ltdb_search() due to a refcounting bug in + * ltdb_lock_read() + */ + +struct search_against_transaction_ctx { + struct ldbtest_ctx *test_ctx; + int res_count; + pid_t child_pid; + struct ldb_dn *basedn; +}; + +static int test_ldb_search_against_transaction_callback2(struct ldb_request *req, + struct ldb_reply *ares) +{ + struct search_against_transaction_ctx *ctx = req->context; + switch (ares->type) { + case LDB_REPLY_ENTRY: + ctx->res_count++; + if (ctx->res_count != 1) { + return LDB_SUCCESS; + } + + break; + + case LDB_REPLY_REFERRAL: + break; + + case LDB_REPLY_DONE: + return ldb_request_done(req, LDB_SUCCESS); + } + + return 0; + +} + +/* + * This purpose of this callback is to trigger a transaction in + * the child process while the all-record lock is held, but before + * we take any locks in the tdb_traverse_read() handler. + * + * In tdb 1.3.12 tdb_traverse_read() take the read transaction lock + * however in ldb 1.1.29 ltdb_search() forgets to take the all-record + * lock (except the very first time) due to a ref-counting bug. + * + */ + +static int test_ldb_search_against_transaction_callback1(struct ldb_request *req, + struct ldb_reply *ares) +{ + int ret; + int pipes[2]; + char buf[2]; + struct search_against_transaction_ctx *ctx = req->context; + switch (ares->type) { + case LDB_REPLY_ENTRY: + break; + + case LDB_REPLY_REFERRAL: + return LDB_SUCCESS; + + case LDB_REPLY_DONE: + return ldb_request_done(req, LDB_SUCCESS); + } + + ret = pipe(pipes); + assert_int_equal(ret, 0); + + ctx->child_pid = fork(); + if (ctx->child_pid == 0) { + TALLOC_CTX *tmp_ctx = NULL; + struct ldb_message *msg; + TALLOC_FREE(ctx->test_ctx->ldb); + TALLOC_FREE(ctx->test_ctx->ev); + ctx->test_ctx->ev = tevent_context_init(ctx->test_ctx); + if (ctx->test_ctx->ev == NULL) { + exit(LDB_ERR_OPERATIONS_ERROR); + } + + ctx->test_ctx->ldb = ldb_init(ctx->test_ctx, + ctx->test_ctx->ev); + if (ctx->test_ctx->ldb == NULL) { + exit(LDB_ERR_OPERATIONS_ERROR); + } + + ret = ldb_connect(ctx->test_ctx->ldb, + ctx->test_ctx->dbpath, 0, NULL); + if (ret != LDB_SUCCESS) { + exit(ret); + } + + tmp_ctx = talloc_new(ctx->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); + } + + msg->dn = ldb_dn_new_fmt(msg, ctx->test_ctx->ldb, + "dc=test"); + if (msg->dn == NULL) { + exit(LDB_ERR_OPERATIONS_ERROR); + } + + ret = ldb_msg_add_string(msg, "cn", "test_cn_val"); + if (ret != 0) { + exit(LDB_ERR_OPERATIONS_ERROR); + } + + ret = ldb_transaction_start(ctx->test_ctx->ldb); + if (ret != 0) { + exit(ret); + } + + ret = write(pipes[1], "GO", 2); + if (ret != 2) { + exit(LDB_ERR_OPERATIONS_ERROR); + } + + ret = ldb_add(ctx->test_ctx->ldb, msg); + if (ret != 0) { + exit(ret); + } + + ret = ldb_transaction_commit(ctx->test_ctx->ldb); + exit(ret); + } + + ret = read(pipes[0], buf, 2); + assert_int_equal(ret, 2); + + /* This search must be unindexed (ie traverse in tdb) */ + ret = ldb_build_search_req(&req, + ctx->test_ctx->ldb, + ctx->test_ctx, + ctx->basedn, + LDB_SCOPE_SUBTREE, + "cn=*", NULL, + NULL, + ctx, + test_ldb_search_against_transaction_callback2, + NULL); + assert_int_equal(ret, 0); + ret = ldb_request(ctx->test_ctx->ldb, req); + + if (ret == LDB_SUCCESS) { + ret = ldb_wait(req->handle, LDB_WAIT_ALL); + } + assert_int_equal(ret, 0); + assert_int_equal(ctx->res_count, 2); + + return LDB_SUCCESS; +} + +static void test_ldb_search_against_transaction(void **state) +{ + struct search_test_ctx *search_test_ctx = talloc_get_type_abort(*state, + struct search_test_ctx); + struct search_against_transaction_ctx + ctx = + { .res_count = 0, + .test_ctx = search_test_ctx->ldb_test_ctx + }; + + int ret; + struct ldb_request *req; + pid_t pid; + int wstatus; + struct ldb_dn *base_search_dn; + + tevent_loop_allow_nesting(search_test_ctx->ldb_test_ctx->ev); + + base_search_dn + = ldb_dn_new_fmt(search_test_ctx, + search_test_ctx->ldb_test_ctx->ldb, + "cn=test_search_cn,%s", + search_test_ctx->base_dn); + assert_non_null(base_search_dn); + + ctx.basedn + = ldb_dn_new_fmt(search_test_ctx, + search_test_ctx->ldb_test_ctx->ldb, + "%s", + search_test_ctx->base_dn); + assert_non_null(ctx.basedn); + + + /* This search must be indexed (ie no traverse in tdb) */ + ret = ldb_build_search_req(&req, + search_test_ctx->ldb_test_ctx->ldb, + search_test_ctx, + base_search_dn, + LDB_SCOPE_BASE, + "cn=*", NULL, + NULL, + &ctx, + test_ldb_search_against_transaction_callback1, + NULL); + assert_int_equal(ret, 0); + ret = ldb_request(search_test_ctx->ldb_test_ctx->ldb, req); + + if (ret == LDB_SUCCESS) { + ret = ldb_wait(req->handle, LDB_WAIT_ALL); + } + assert_int_equal(ret, 0); + assert_int_equal(ctx.res_count, 2); + + pid = waitpid(ctx.child_pid, &wstatus, 0); + assert_int_equal(pid, ctx.child_pid); + + assert_true(WIFEXITED(wstatus)); + + assert_int_equal(WEXITSTATUS(wstatus), 0); + + +} + static int ldb_case_test_setup(void **state) { int ret; @@ -1516,6 +1756,9 @@ int main(int argc, const char **argv) cmocka_unit_test_setup_teardown(test_search_match_basedn, ldb_search_test_setup, ldb_search_test_teardown), + cmocka_unit_test_setup_teardown(test_ldb_search_against_transaction, + 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), diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py index c2b3c89..4d290ef 100755 --- a/lib/ldb/tests/python/api.py +++ b/lib/ldb/tests/python/api.py @@ -5,19 +5,21 @@ import os from unittest import TestCase import sys - +import gc +import time import ldb +import shutil PY3 = sys.version_info > (3, 0) -def filename(): +def tempdir(): import tempfile try: dir_prefix = os.path.join(os.environ["SELFTEST_PREFIX"], "tmp") except KeyError: dir_prefix = None - return tempfile.mktemp(dir=dir_prefix) + return tempfile.mkdtemp(dir=dir_prefix) class NoContextTests(TestCase): @@ -44,15 +46,25 @@ class NoContextTests(TestCase): class SimpleLdb(TestCase): + def setUp(self): + super(SimpleLdb, self).setUp() + self.testdir = tempdir() + self.filename = os.path.join(self.testdir, "test.ldb") + self.ldb = ldb.Ldb(self.filename) + + def tearDown(self): + shutil.rmtree(self.testdir) + super(SimpleLdb, self).setUp() + def test_connect(self): - ldb.Ldb(filename()) + ldb.Ldb(self.filename) def test_connect_none(self): ldb.Ldb() def test_connect_later(self): x = ldb.Ldb() - x.connect(filename()) + x.connect(self.filename) def test_repr(self): x = ldb.Ldb() @@ -67,7 +79,7 @@ class SimpleLdb(TestCase): self.assertEqual([], x.modules()) def test_modules_tdb(self): - x = ldb.Ldb(filename()) + x = ldb.Ldb(self.filename) self.assertEqual("[<ldb module 'tdb'>]", repr(x.modules())) def test_firstmodule_none(self): @@ -75,48 +87,53 @@ class SimpleLdb(TestCase): self.assertEqual(x.firstmodule, None) def test_firstmodule_tdb(self): - x = ldb.Ldb(filename()) + x = ldb.Ldb(self.filename) mod = x.firstmodule self.assertEqual(repr(mod), "<ldb module 'tdb'>") def test_search(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertEqual(len(l.search()), 0) def test_search_controls(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertEqual(len(l.search(controls=["paged_results:0:5"])), 0) def test_search_attrs(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertEqual(len(l.search(ldb.Dn(l, ""), ldb.SCOPE_SUBTREE, "(dc=*)", ["dc"])), 0) def test_search_string_dn(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertEqual(len(l.search("", ldb.SCOPE_SUBTREE, "(dc=*)", ["dc"])), 0) def test_search_attr_string(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertRaises(TypeError, l.search, attrs="dc") self.assertRaises(TypeError, l.search, attrs=b"dc") def test_opaque(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) l.set_opaque("my_opaque", l) self.assertTrue(l.get_opaque("my_opaque") is not None) self.assertEqual(None, l.get_opaque("unknown")) - def test_search_scope_base(self): - l = ldb.Ldb(filename()) + def test_search_scope_base_empty_db(self): + l = ldb.Ldb(self.filename) + self.assertEqual(len(l.search(ldb.Dn(l, "dc=foo1"), + ldb.SCOPE_BASE)), 0) + + def test_search_scope_onelevel_empty_db(self): + l = ldb.Ldb(self.filename) self.assertEqual(len(l.search(ldb.Dn(l, "dc=foo1"), ldb.SCOPE_ONELEVEL)), 0) def test_delete(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertRaises(ldb.LdbError, lambda: l.delete(ldb.Dn(l, "dc=foo2"))) def test_delete_w_unhandled_ctrl(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) m = ldb.Message() m.dn = ldb.Dn(l, "dc=foo1") m["b"] = [b"a"] @@ -125,7 +142,7 @@ class SimpleLdb(TestCase): l.delete(m.dn) def test_contains(self): - name = filename() + name = self.filename l = ldb.Ldb(name) self.assertFalse(ldb.Dn(l, "dc=foo3") in l) l = ldb.Ldb(name) @@ -140,23 +157,23 @@ class SimpleLdb(TestCase): l.delete(m.dn) def test_get_config_basedn(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertEqual(None, l.get_config_basedn()) def test_get_root_basedn(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertEqual(None, l.get_root_basedn()) def test_get_schema_basedn(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertEqual(None, l.get_schema_basedn()) def test_get_default_basedn(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) self.assertEqual(None, l.get_default_basedn()) def test_add(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) m = ldb.Message() m.dn = ldb.Dn(l, "dc=foo4") m["bla"] = b"bla" @@ -168,7 +185,7 @@ class SimpleLdb(TestCase): l.delete(ldb.Dn(l, "dc=foo4")) def test_search_iterator(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) s = l.search_iterator() s.abandon() try: @@ -268,7 +285,7 @@ class SimpleLdb(TestCase): l.delete(ldb.Dn(l, "dc=foo5")) def test_add_text(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) m = ldb.Message() m.dn = ldb.Dn(l, "dc=foo4") m["bla"] = "bla" @@ -280,7 +297,7 @@ class SimpleLdb(TestCase): l.delete(ldb.Dn(l, "dc=foo4")) def test_add_w_unhandled_ctrl(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) m = ldb.Message() m.dn = ldb.Dn(l, "dc=foo4") m["bla"] = b"bla" @@ -288,7 +305,7 @@ class SimpleLdb(TestCase): self.assertRaises(ldb.LdbError, lambda: l.add(m,["search_options:1:2"])) def test_add_dict(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) m = {"dn": ldb.Dn(l, "dc=foo5"), "bla": b"bla"} self.assertEqual(len(l.search()), 0) @@ -299,7 +316,7 @@ class SimpleLdb(TestCase): l.delete(ldb.Dn(l, "dc=foo5")) def test_add_dict_text(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) m = {"dn": ldb.Dn(l, "dc=foo5"), "bla": "bla"} self.assertEqual(len(l.search()), 0) @@ -310,7 +327,7 @@ class SimpleLdb(TestCase): l.delete(ldb.Dn(l, "dc=foo5")) def test_add_dict_string_dn(self): - l = ldb.Ldb(filename()) + l = ldb.Ldb(self.filename) m = {"dn": "dc=foo6", "bla": b"bla"} self.assertEqual(len(l.search()), 0) l.add(m) @@ -320,7 +337,7 @@ class SimpleLdb(TestCase): l.delete(ldb.Dn(l, "dc=foo6")) def test_add_dict_bytes_dn(self): - l = ldb.Ldb(filename()) -- Samba Shared Repository