Releases, locking and ldb
Andrew Bartlett
abartlet at samba.org
Fri Jun 30 11:03:17 UTC 2017
On Fri, 2017-06-30 at 08:26 +0200, Stefan Metzmacher wrote:
> Am 30.06.2017 um 07:40 schrieb Stefan Metzmacher via samba-technical:
> > Am 28.06.2017 um 23:54 schrieb Andrew Bartlett:
> > > On Wed, 2017-06-28 at 21:47 +0200, Stefan Metzmacher wrote:
> > > > Hi Andrew,
> > > >
> > > > > > The only good news is that I have confirmed that 4.6 fails with 1.1.31
> > > > > > for 'make test TESTS=fsmo', and that the proposed 1.1.32 fixes this
> > > > > > specific test.
> > > > > >
> > > > > > This took me the whole work day, so the lack of a clear resolution is a
> > > > > > little deflating, but I hope this helps.
> > > >
> > > > Thanks for all these tests!
> > > >
> > > > > I think the best course of action is to say the supported version of
> > > > > ldb for 4.6 is the version it shipped with, nothing else. There is
> > > > > nothing gained by including the more recent patches, only risk.
> > > > >
> > > > > I would suggest to un-release the 1.1.30 and 1.1.31 versions from the
> > > > > download page to avoid packaging by distributions (replace with a note
> > > > > saying they are withdrawn).
> > > >
> > > > I also thought about something similar.
> > >
> > > Good.
> > >
> > > > I'd also try to add a maxversion and/or a blacklist
> > > > to CHECK_BUNDLED_SYSTEM_PKG() and put that to 4.6 and 4.5.
> > >
> > > Thanks.
> >
> > Can you verify the attached patches work on 4.5,
> > we should try to get them into 4.5.11 next week.
> >
> > I'm currently running autobuild with it for 4.6 and 4.5.
> > But it would be good if you could verify the real world
> > setup with an incompatible version in the system.
> > Maybe changing 'VERSION' in lib/ldb/wscript to 1.1.10
> > and blacklist 1.1.27 or set max version to 1.1.26
> > should be able to verify the protection works.
> >
> > I already did some basic checks using hacks
> > in CHECK_ZLIB().
> >
> > > > I'll also try to see if I get something like your
> > > > ldb.h #ifdef _SAMBA_BUILD_
> > > > hack also to work.
> > >
> > > OK. BTW putting it in ldb_module.h was deliberate, to avoid it being
> > > in a header more public than strictly required.
> >
> > Thanks for reminding me!
> >
> > I'm currently using abartlet/ldb-safe-locking-11
> > (42adb5b979cfebc86fc461dd69f1a5c7ece3f109)
> > in order to prepare the final branch.
>
> Can you test the attaches magic protects 4.5.10
Thank you so much for following up on this.
To be clear, I can't promise anything until Monday NZ time. If you can
clear the WRONG_TEST and SIGN_OFF markers between now and then, and let
me know if I can land the patches after your specified tests, I would
appreciate it.
Feel free to include the tdb and ldb_tdb patches attached here from the
no-nested transactions branch if you like, I'm currently running
private autobuilds it the Catalyst Cloud to check.
Perhaps I'm tempting fate too much making changes after we got it all
working, but I've attached my current thoughts. I wish I could do more
to actually remove the nested transactions, but now is not the time.
Once I get this out of the way, I'll focus on writing up more WHATSNEW.
My colleagues have given me a list of things to flesh out already, it
has been a busy few months :-)
Thanks,
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
-------------- next part --------------
From 400c58f4bd9dc1480d44a463319614c9ef1f05bd Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 30 Jun 2017 06:21:32 +0200
Subject: [PATCH 01/35] wafsamba: add maxversion and version_blacklist to
CHECK_BUNDLED_SYSTEM[_PKG]()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12859
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
buildtools/wafsamba/samba_bundled.py | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/buildtools/wafsamba/samba_bundled.py b/buildtools/wafsamba/samba_bundled.py
index ea88807..aa6199e 100644
--- a/buildtools/wafsamba/samba_bundled.py
+++ b/buildtools/wafsamba/samba_bundled.py
@@ -110,6 +110,7 @@ def LIB_MUST_BE_PRIVATE(conf, libname):
@conf
def CHECK_BUNDLED_SYSTEM_PKG(conf, libname, minversion='0.0.0',
+ maxversion=None, version_blacklist=[],
onlyif=None, implied_deps=None, pkg=None):
'''check if a library is available as a system library.
@@ -117,12 +118,15 @@ def CHECK_BUNDLED_SYSTEM_PKG(conf, libname, minversion='0.0.0',
'''
return conf.CHECK_BUNDLED_SYSTEM(libname,
minversion=minversion,
+ maxversion=maxversion,
+ version_blacklist=version_blacklist,
onlyif=onlyif,
implied_deps=implied_deps,
pkg=pkg)
@conf
def CHECK_BUNDLED_SYSTEM(conf, libname, minversion='0.0.0',
+ maxversion=None, version_blacklist=[],
checkfunctions=None, headers=None, checkcode=None,
onlyif=None, implied_deps=None,
require_headers=True, pkg=None, set_target=True):
@@ -181,16 +185,29 @@ def CHECK_BUNDLED_SYSTEM(conf, libname, minversion='0.0.0',
minversion = minimum_library_version(conf, libname, minversion)
msg = 'Checking for system %s' % libname
+ msg_ver = []
if minversion != '0.0.0':
- msg += ' >= %s' % minversion
+ msg_ver.append('>=%s' % minversion)
+ if maxversion is not None:
+ msg_ver.append('<=%s' % maxversion)
+ for v in version_blacklist:
+ msg_ver.append('!=%s' % v)
+ if msg_ver != []:
+ msg += " (%s)" % (" ".join(msg_ver))
uselib_store=libname.upper()
if pkg is None:
pkg = libname
+ version_checks = '%s >= %s' % (pkg, minversion)
+ if maxversion is not None:
+ version_checks += ' %s <= %s' % (pkg, maxversion)
+ for v in version_blacklist:
+ version_checks += ' %s != %s' % (pkg, v)
+
# try pkgconfig first
if (conf.CHECK_CFG(package=pkg,
- args='"%s >= %s" --cflags --libs' % (pkg, minversion),
+ args='"%s" --cflags --libs' % (version_checks),
msg=msg, uselib_store=uselib_store) and
check_functions_headers_code()):
if set_target:
--
2.9.4
From 88e0e7fbb02788cb1d609535dc90409c705dbed2 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 31 Mar 2017 17:34:13 +1300
Subject: [PATCH 02/35] tdb: Remove locking from tdb_traverse_read()
This restores the original intent of tdb_traverse_read() in
7dd31288a701d772e45b1960ac4ce4cc1be782ed
This is needed to avoid a deadlock with tdb_lockall() and the
transaction start, as ldb_tdb should take the allrecord lock during a
search (which calls tdb_traverse), and can otherwise deadlock against
a transaction starting in another process
We add a test to show that a transaction can now start while a read
traverse is in progress
This allows more operations to happen in parallel. The blocking point
is moved to the prepare commit.
This in turn permits a roughly doubling of unindexed search
performance, because currently ldb_tdb omits to take the lock due to
an unrelated bug, but taking the allrecord lock triggers the
above-mentioned deadlock.
This behaviour was added in 251aaafe3a9213118ac3a92def9ab2104c40d12a for
Solaris 10 in 2005. But the run-fcntl-deadlock test works also on Solaris 10,
see https://lists.samba.org/archive/samba-technical/2017-April/119876.html.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/tdb/common/traverse.c | 10 +---------
lib/tdb/test/run-nested-traverse.c | 31 +++++++++++++++++++++++++++----
2 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/lib/tdb/common/traverse.c b/lib/tdb/common/traverse.c
index f33ef34..f62306e 100644
--- a/lib/tdb/common/traverse.c
+++ b/lib/tdb/common/traverse.c
@@ -244,7 +244,7 @@ out:
/*
- a read style traverse - temporarily marks the db read only
+ a read style traverse - temporarily marks each record read only
*/
_PUBLIC_ int tdb_traverse_read(struct tdb_context *tdb,
tdb_traverse_func fn, void *private_data)
@@ -252,19 +252,11 @@ _PUBLIC_ int tdb_traverse_read(struct tdb_context *tdb,
struct tdb_traverse_lock tl = { NULL, 0, 0, F_RDLCK };
int ret;
- /* we need to get a read lock on the transaction lock here to
- cope with the lock ordering semantics of solaris10 */
- if (tdb_transaction_lock(tdb, F_RDLCK, TDB_LOCK_WAIT)) {
- return -1;
- }
-
tdb->traverse_read++;
tdb_trace(tdb, "tdb_traverse_read_start");
ret = tdb_traverse_internal(tdb, fn, private_data, &tl);
tdb->traverse_read--;
- tdb_transaction_unlock(tdb, F_RDLCK);
-
return ret;
}
diff --git a/lib/tdb/test/run-nested-traverse.c b/lib/tdb/test/run-nested-traverse.c
index 22ee3e2..aeaa085 100644
--- a/lib/tdb/test/run-nested-traverse.c
+++ b/lib/tdb/test/run-nested-traverse.c
@@ -41,7 +41,30 @@ static int traverse2(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
return 0;
}
-static int traverse1(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
+static int traverse1r(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
+ void *p)
+{
+ ok1(correct_key(key));
+ ok1(correct_data(data));
+ ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb))
+ == SUCCESS);
+ ok1(external_agent_operation(agent, STORE, tdb_name(tdb))
+ == SUCCESS);
+ ok1(external_agent_operation(agent, TRANSACTION_COMMIT, tdb_name(tdb))
+ == WOULD_HAVE_BLOCKED);
+ tdb_traverse(tdb, traverse2, NULL);
+
+ /* That should *not* release the all-records lock! */
+ ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb))
+ == SUCCESS);
+ ok1(external_agent_operation(agent, STORE, tdb_name(tdb))
+ == SUCCESS);
+ ok1(external_agent_operation(agent, TRANSACTION_COMMIT, tdb_name(tdb))
+ == WOULD_HAVE_BLOCKED);
+ return 0;
+}
+
+static int traverse1w(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
void *p)
{
ok1(correct_key(key));
@@ -50,7 +73,7 @@ static int traverse1(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
== WOULD_HAVE_BLOCKED);
tdb_traverse(tdb, traverse2, NULL);
- /* That should *not* release the transaction lock! */
+ /* That should *not* release the all-records lock! */
ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb))
== WOULD_HAVE_BLOCKED);
return 0;
@@ -80,8 +103,8 @@ int main(int argc, char *argv[])
data.dsize = strlen("world");
ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0);
- tdb_traverse(tdb, traverse1, NULL);
- tdb_traverse_read(tdb, traverse1, NULL);
+ tdb_traverse(tdb, traverse1w, NULL);
+ tdb_traverse_read(tdb, traverse1r, NULL);
tdb_close(tdb);
return exit_status();
--
2.9.4
From a65377d1f883ffef1c290bbd548e160572c1107a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 27 Apr 2017 08:51:08 +1200
Subject: [PATCH 03/35] tdb: Improve documentation for tdb_transaction_start()
It now references the TDB_ALLOW_NESTING and TDB_DISALLOW_NESTING flags
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/tdb/include/tdb.h | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/tdb/include/tdb.h b/lib/tdb/include/tdb.h
index e86d267..4bfa1a4 100644
--- a/lib/tdb/include/tdb.h
+++ b/lib/tdb/include/tdb.h
@@ -656,10 +656,13 @@ void *tdb_get_logging_private(struct tdb_context *tdb);
* All operations after the transaction start can either be committed with
* tdb_transaction_commit() or cancelled with tdb_transaction_cancel().
*
- * If you call tdb_transaction_start() again on the same tdb context while a
- * transaction is in progress, then the same transaction buffer is re-used. The
- * number of tdb_transaction_{commit,cancel} operations must match the number
- * of successful tdb_transaction_start() calls.
+ * If (the default) TDB_ALLOW_NESTING was specified or
+ * TDB_DISALLOW_NESTING was not specified as a flag via tdb_open() or
+ * tdb_open_ex(), you call tdb_transaction_start() again on the same
+ * tdb context while a transaction is in progress, then the same
+ * transaction buffer is re-used. The number of
+ * tdb_transaction_{commit,cancel} operations must match the number of
+ * successful tdb_transaction_start() calls.
*
* Note that transactions are by default disk synchronous, and use a recover
* area in the database to automatically recover the database on the next open
--
2.9.4
From d2e0ab98362b7122a359dcaf13151286ed18c2f7 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 27 Apr 2017 08:34:56 +1200
Subject: [PATCH 04/35] tdb: Add new function tdb_transaction_active()
This will allow callers to avoid their own reference counting of transactions.
Additionally, this will always line up with the acutal transaction state, even
in the error cases where tdb can cancel the transaction
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/tdb/common/transaction.c | 8 ++++++++
lib/tdb/include/tdb.h | 18 ++++++++++++++++++
lib/tdb/test/run-traverse-in-transaction.c | 3 +++
3 files changed, 29 insertions(+)
diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index 420e754..9b975ea 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -412,6 +412,14 @@ static const struct tdb_methods transaction_methods = {
transaction_expand_file,
};
+/*
+ * Is a transaction currently active on this context?
+ *
+ */
+_PUBLIC_ bool tdb_transaction_active(struct tdb_context *tdb)
+{
+ return (tdb->transaction != NULL);
+}
/*
start a tdb transaction. No token is returned, as only a single
diff --git a/lib/tdb/include/tdb.h b/lib/tdb/include/tdb.h
index 4bfa1a4..89bc583 100644
--- a/lib/tdb/include/tdb.h
+++ b/lib/tdb/include/tdb.h
@@ -651,6 +651,24 @@ tdb_log_func tdb_log_fn(struct tdb_context *tdb);
void *tdb_get_logging_private(struct tdb_context *tdb);
/**
+ * @brief Is a transaction active?
+ *
+ * It is helpful for the application to know if a transaction is
+ * active, rather than needing to maintain an application-level reference
+ * count.
+ *
+ * @param[in] tdb The database to start the transaction.
+ *
+ * @return true if there is a transaction active, false otherwise
+ *
+ * @see tdb_transaction_start()
+ * @see tdb_transaction_prepare_commit()
+ * @see tdb_transaction_commit()
+ * @see tdb_transaction_cancel()
+ */
+bool tdb_transaction_active(struct tdb_context *tdb);
+
+/**
* @brief Start a transaction.
*
* All operations after the transaction start can either be committed with
diff --git a/lib/tdb/test/run-traverse-in-transaction.c b/lib/tdb/test/run-traverse-in-transaction.c
index 17d6412..d187b9b 100644
--- a/lib/tdb/test/run-traverse-in-transaction.c
+++ b/lib/tdb/test/run-traverse-in-transaction.c
@@ -63,7 +63,9 @@ int main(int argc, char *argv[])
ok1(external_agent_operation(agent, OPEN, tdb_name(tdb)) == SUCCESS);
+ ok1(tdb_transaction_active(tdb) == 0);
ok1(tdb_transaction_start(tdb) == 0);
+ ok1(tdb_transaction_active(tdb) == 1);
ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb))
== WOULD_HAVE_BLOCKED);
tdb_traverse(tdb, traverse, NULL);
@@ -77,6 +79,7 @@ int main(int argc, char *argv[])
ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb))
== WOULD_HAVE_BLOCKED);
ok1(tdb_transaction_commit(tdb) == 0);
+ ok1(tdb_transaction_active(tdb) == 0);
/* Now we should be fine. */
ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb))
== SUCCESS);
--
2.9.4
From 5a089f4e56df607867a458fb172c6ebf89fa6747 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 11 Apr 2017 17:27:33 +0200
Subject: [PATCH 05/35] tdb: version 1.3.14
* allow tdb_traverse_read before tdb_transaction[_prepare]_commit()
* Improve documentation for tdb_transaction_start()
* Add new function tdb_transaction_active()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
lib/tdb/ABI/tdb-1.3.14.sigs | 71 +++++++++++++++++++++++++++++++++++++++++++++
lib/tdb/wscript | 2 +-
2 files changed, 72 insertions(+), 1 deletion(-)
create mode 100644 lib/tdb/ABI/tdb-1.3.14.sigs
diff --git a/lib/tdb/ABI/tdb-1.3.14.sigs b/lib/tdb/ABI/tdb-1.3.14.sigs
new file mode 100644
index 0000000..61ce5e6
--- /dev/null
+++ b/lib/tdb/ABI/tdb-1.3.14.sigs
@@ -0,0 +1,71 @@
+tdb_add_flags: void (struct tdb_context *, unsigned int)
+tdb_append: int (struct tdb_context *, TDB_DATA, TDB_DATA)
+tdb_chainlock: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_mark: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_nonblock: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_read: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_read_nonblock: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_unmark: int (struct tdb_context *, TDB_DATA)
+tdb_chainunlock: int (struct tdb_context *, TDB_DATA)
+tdb_chainunlock_read: int (struct tdb_context *, TDB_DATA)
+tdb_check: int (struct tdb_context *, int (*)(TDB_DATA, TDB_DATA, void *), void *)
+tdb_close: int (struct tdb_context *)
+tdb_delete: int (struct tdb_context *, TDB_DATA)
+tdb_dump_all: void (struct tdb_context *)
+tdb_enable_seqnum: void (struct tdb_context *)
+tdb_error: enum TDB_ERROR (struct tdb_context *)
+tdb_errorstr: const char *(struct tdb_context *)
+tdb_exists: int (struct tdb_context *, TDB_DATA)
+tdb_fd: int (struct tdb_context *)
+tdb_fetch: TDB_DATA (struct tdb_context *, TDB_DATA)
+tdb_firstkey: TDB_DATA (struct tdb_context *)
+tdb_freelist_size: int (struct tdb_context *)
+tdb_get_flags: int (struct tdb_context *)
+tdb_get_logging_private: void *(struct tdb_context *)
+tdb_get_seqnum: int (struct tdb_context *)
+tdb_hash_size: int (struct tdb_context *)
+tdb_increment_seqnum_nonblock: void (struct tdb_context *)
+tdb_jenkins_hash: unsigned int (TDB_DATA *)
+tdb_lock_nonblock: int (struct tdb_context *, int, int)
+tdb_lockall: int (struct tdb_context *)
+tdb_lockall_mark: int (struct tdb_context *)
+tdb_lockall_nonblock: int (struct tdb_context *)
+tdb_lockall_read: int (struct tdb_context *)
+tdb_lockall_read_nonblock: int (struct tdb_context *)
+tdb_lockall_unmark: int (struct tdb_context *)
+tdb_log_fn: tdb_log_func (struct tdb_context *)
+tdb_map_size: size_t (struct tdb_context *)
+tdb_name: const char *(struct tdb_context *)
+tdb_nextkey: TDB_DATA (struct tdb_context *, TDB_DATA)
+tdb_null: dptr = 0xXXXX, dsize = 0
+tdb_open: struct tdb_context *(const char *, int, int, int, mode_t)
+tdb_open_ex: struct tdb_context *(const char *, int, int, int, mode_t, const struct tdb_logging_context *, tdb_hash_func)
+tdb_parse_record: int (struct tdb_context *, TDB_DATA, int (*)(TDB_DATA, TDB_DATA, void *), void *)
+tdb_printfreelist: int (struct tdb_context *)
+tdb_remove_flags: void (struct tdb_context *, unsigned int)
+tdb_reopen: int (struct tdb_context *)
+tdb_reopen_all: int (int)
+tdb_repack: int (struct tdb_context *)
+tdb_rescue: int (struct tdb_context *, void (*)(TDB_DATA, TDB_DATA, void *), void *)
+tdb_runtime_check_for_robust_mutexes: bool (void)
+tdb_set_logging_function: void (struct tdb_context *, const struct tdb_logging_context *)
+tdb_set_max_dead: void (struct tdb_context *, int)
+tdb_setalarm_sigptr: void (struct tdb_context *, volatile sig_atomic_t *)
+tdb_store: int (struct tdb_context *, TDB_DATA, TDB_DATA, int)
+tdb_storev: int (struct tdb_context *, TDB_DATA, const TDB_DATA *, int, int)
+tdb_summary: char *(struct tdb_context *)
+tdb_transaction_active: bool (struct tdb_context *)
+tdb_transaction_cancel: int (struct tdb_context *)
+tdb_transaction_commit: int (struct tdb_context *)
+tdb_transaction_prepare_commit: int (struct tdb_context *)
+tdb_transaction_start: int (struct tdb_context *)
+tdb_transaction_start_nonblock: int (struct tdb_context *)
+tdb_transaction_write_lock_mark: int (struct tdb_context *)
+tdb_transaction_write_lock_unmark: int (struct tdb_context *)
+tdb_traverse: int (struct tdb_context *, tdb_traverse_func, void *)
+tdb_traverse_read: int (struct tdb_context *, tdb_traverse_func, void *)
+tdb_unlock: int (struct tdb_context *, int, int)
+tdb_unlockall: int (struct tdb_context *)
+tdb_unlockall_read: int (struct tdb_context *)
+tdb_validate_freelist: int (struct tdb_context *, int *)
+tdb_wipe_all: int (struct tdb_context *)
diff --git a/lib/tdb/wscript b/lib/tdb/wscript
index 09bc0a3..4782550 100644
--- a/lib/tdb/wscript
+++ b/lib/tdb/wscript
@@ -1,7 +1,7 @@
#!/usr/bin/env python
APPNAME = 'tdb'
-VERSION = '1.3.13'
+VERSION = '1.3.14'
blddir = 'bin'
--
2.9.4
From 7254ab9e2fd77daa97f63c5e3ba1909e853c4236 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 30 Mar 2017 12:03:17 +1300
Subject: [PATCH 06/35] ldb:tdb: Ensure we correctly decrement
ltdb->read_lock_count
If we do not do this, then we never take the all record lock, and instead do a lock
for every record as we go, which is very slow during a large search
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/ldb_tdb/ldb_tdb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index f470e02..ad15a5e 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -119,6 +119,7 @@ int ltdb_unlock_read(struct ldb_module *module)
struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
if (ltdb->in_transaction == 0 && ltdb->read_lock_count == 1) {
tdb_unlockall_read(ltdb->tdb);
+ ltdb->read_lock_count--;
return 0;
}
ltdb->read_lock_count--;
--
2.9.4
From b16533a01209fb945195bb7a92f4a76652d149ea Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Jun 2017 13:56:46 +1200
Subject: [PATCH 07/35] ldb:tests: don't assert the results before doing the
final search finished
This is required to pass the test in future, because
otherwise the clean up will fail because we hold locks.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/tests/ldb_mod_op_test.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index fc4b134..dedad15 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -1335,7 +1335,7 @@ static int test_ldb_search_against_transaction_callback2(struct ldb_request *req
static int test_ldb_search_against_transaction_callback1(struct ldb_request *req,
struct ldb_reply *ares)
{
- int ret;
+ int ret, ret2;
int pipes[2];
char buf[2];
struct search_against_transaction_ctx *ctx = req->context;
@@ -1430,13 +1430,18 @@ static int test_ldb_search_against_transaction_callback1(struct ldb_request *req
ctx,
test_ldb_search_against_transaction_callback2,
NULL);
- assert_int_equal(ret, 0);
- ret = ldb_request(ctx->test_ctx->ldb, req);
+ /*
+ * we don't assert on these return codes until after the search is
+ * finished, or the clean up will fail because we hold locks.
+ */
- if (ret == LDB_SUCCESS) {
- ret = ldb_wait(req->handle, LDB_WAIT_ALL);
+ ret2 = ldb_request(ctx->test_ctx->ldb, req);
+
+ if (ret2 == LDB_SUCCESS) {
+ ret2 = ldb_wait(req->handle, LDB_WAIT_ALL);
}
assert_int_equal(ret, 0);
+ assert_int_equal(ret2, 0);
assert_int_equal(ctx->res_count, 2);
return LDB_SUCCESS;
--
2.9.4
From 95a4f602a04ee19f20956a96ed8c8ac5184cb17d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 25 Apr 2017 22:33:53 +1200
Subject: [PATCH 08/35] ldb:tests: Show that writes do not appear during an
ldb_search()
A modify or rename during a search must not cause a search to change
output, and attributes having an index should in particular not see
any change in behaviour in this respect
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/tests/ldb_mod_op_test.c | 349 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 349 insertions(+)
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index dedad15..475138e 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -1510,6 +1510,343 @@ static void test_ldb_search_against_transaction(void **state)
}
+/*
+ * This test is also complex.
+ * The purpose is to test if a modify can occur during an ldb_search()
+ * This would be a failure if if in process
+ * (1) and (2):
+ * - (1) ltdb_search() starts and calls back for one entry
+ * - (2) one of the entries to be matched is modified
+ * - (1) the indexed search tries to return the modified entry, but
+ * it is no longer found, either:
+ * - despite it still matching (dn changed)
+ * - it no longer matching (attrs changed)
+ *
+ * We also try un-indexed to show that the behaviour differs on this
+ * point, which it should not (an index should only impact search
+ * speed).
+ */
+
+struct modify_during_search_test_ctx {
+ struct ldbtest_ctx *test_ctx;
+ int res_count;
+ pid_t child_pid;
+ struct ldb_dn *basedn;
+ bool got_cn;
+ bool got_2_cn;
+ bool rename;
+};
+
+/*
+ * This purpose of this callback is to trigger a write in
+ * the child process while a search is in progress.
+ *
+ * In tdb 1.3.12 tdb_traverse_read() take the read transaction lock
+ * however in ldb 1.1.31 ltdb_search() forgets to take the all-record
+ * lock (except the very first time) due to a ref-counting bug.
+ *
+ * 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_during_search_callback1(struct ldb_request *req,
+ struct ldb_reply *ares)
+{
+ int ret;
+ int pipes[2];
+ char buf[2];
+ struct modify_during_search_test_ctx *ctx = req->context;
+ switch (ares->type) {
+ case LDB_REPLY_ENTRY:
+ {
+ const struct ldb_val *cn_val
+ = ldb_dn_get_component_val(ares->message->dn, 0);
+ const char *cn = (char *)cn_val->data;
+ ctx->res_count++;
+ if (strcmp(cn, "test_search_cn") == 0) {
+ ctx->got_cn = true;
+ } else if (strcmp(cn, "test_search_2_cn") == 0) {
+ ctx->got_2_cn = true;
+ }
+ if (ctx->res_count == 2) {
+ return LDB_SUCCESS;
+ }
+ 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 && ctx->rename) {
+ TALLOC_CTX *tmp_ctx = NULL;
+ struct ldb_dn *dn, *new_dn;
+ 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);
+ }
+
+ if (ctx->got_cn) {
+ /* Modify the other one */
+ dn = ldb_dn_new_fmt(tmp_ctx, ctx->test_ctx->ldb,
+ "cn=test_search_2_cn,"
+ "dc=search_test_entry");
+ } else {
+ dn = ldb_dn_new_fmt(tmp_ctx, ctx->test_ctx->ldb,
+ "cn=test_search_cn,"
+ "dc=search_test_entry");
+ }
+ if (dn == NULL) {
+ exit(LDB_ERR_OPERATIONS_ERROR);
+ }
+
+ new_dn = ldb_dn_new_fmt(tmp_ctx, ctx->test_ctx->ldb,
+ "cn=test_search_cn_renamed,"
+ "dc=search_test_entry");
+ if (new_dn == NULL) {
+ exit(LDB_ERR_OPERATIONS_ERROR);
+ }
+
+ ret = ldb_transaction_start(ctx->test_ctx->ldb);
+ if (ret != 0) {
+ exit(ret);
+ }
+
+ if (write(pipes[1], "GO", 2) != 2) {
+ exit(LDB_ERR_OPERATIONS_ERROR);
+ }
+
+ ret = ldb_rename(ctx->test_ctx->ldb, dn, new_dn);
+ if (ret != 0) {
+ exit(ret);
+ }
+
+ ret = ldb_transaction_commit(ctx->test_ctx->ldb);
+ exit(ret);
+
+ } else if (ctx->child_pid == 0) {
+ TALLOC_CTX *tmp_ctx = NULL;
+ struct ldb_message *msg;
+ struct ldb_message_element *el;
+ 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);
+ }
+
+ if (ctx->got_cn) {
+ /* Modify the other one */
+ msg->dn = ldb_dn_new_fmt(msg, ctx->test_ctx->ldb,
+ "cn=test_search_2_cn,"
+ "dc=search_test_entry");
+ } else {
+ msg->dn = ldb_dn_new_fmt(msg, ctx->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(ctx->test_ctx->ldb);
+ if (ret != 0) {
+ exit(ret);
+ }
+
+ if (write(pipes[1], "GO", 2) != 2) {
+ exit(LDB_ERR_OPERATIONS_ERROR);
+ }
+
+ ret = ldb_modify(ctx->test_ctx->ldb, msg);
+ if (ret != 0) {
+ exit(ret);
+ }
+
+ ret = ldb_transaction_commit(ctx->test_ctx->ldb);
+ exit(ret);
+ }
+
+ /*
+ * With TDB 1.3.13 and before "tdb: Remove locking from tdb_traverse_read()"
+ * we will hang here because the child process can not proceed to
+ * sending the "GO" as it is blocked at ldb_transaction_start().
+ */
+
+ ret = read(pipes[0], buf, 2);
+ assert_int_equal(ret, 2);
+
+ sleep(3);
+
+ return LDB_SUCCESS;
+}
+
+static void test_ldb_modify_during_search(void **state, bool add_index,
+ bool rename)
+{
+ struct search_test_ctx *search_test_ctx = talloc_get_type_abort(*state,
+ struct search_test_ctx);
+ struct modify_during_search_test_ctx
+ ctx =
+ { .res_count = 0,
+ .test_ctx = search_test_ctx->ldb_test_ctx,
+ .rename = rename
+ };
+
+ int ret;
+ struct ldb_request *req;
+ pid_t pid;
+ int wstatus;
+
+ if (add_index) {
+ struct ldb_message *msg;
+ struct ldb_dn *indexlist = ldb_dn_new(search_test_ctx,
+ search_test_ctx->ldb_test_ctx->ldb,
+ "@INDEXLIST");
+ assert_non_null(indexlist);
+
+ msg = ldb_msg_new(search_test_ctx);
+ assert_non_null(msg);
+
+ msg->dn = indexlist;
+
+ ret = ldb_msg_add_string(msg, "@IDXATTR", "cn");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_add(search_test_ctx->ldb_test_ctx->ldb,
+ msg);
+
+ assert_int_equal(ret, LDB_SUCCESS);
+ }
+
+ tevent_loop_allow_nesting(search_test_ctx->ldb_test_ctx->ev);
+
+ 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 over multiple items, and should include
+ * the new name after a rename, to show that it would match
+ * both before and after that modify
+ */
+ ret = ldb_build_search_req(&req,
+ search_test_ctx->ldb_test_ctx->ldb,
+ search_test_ctx,
+ ctx.basedn,
+ LDB_SCOPE_SUBTREE,
+ "(&(!(filterAttr=*))"
+ "(|(cn=test_search_cn_renamed)"
+ "(cn=test_search_cn)"
+ "(cn=test_search_2_cn)"
+ "))",
+ NULL,
+ NULL,
+ &ctx,
+ test_ldb_modify_during_search_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);
+ assert_int_equal(ctx.got_cn, true);
+ assert_int_equal(ctx.got_2_cn, true);
+
+ 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 void test_ldb_modify_during_indexed_search(void **state)
+{
+ return test_ldb_modify_during_search(state, true, false);
+}
+
+static void test_ldb_modify_during_unindexed_search(void **state)
+{
+ return test_ldb_modify_during_search(state, false, false);
+}
+
+static void test_ldb_rename_during_indexed_search(void **state)
+{
+ return test_ldb_modify_during_search(state, true, true);
+}
+
+static void test_ldb_rename_during_unindexed_search(void **state)
+{
+ return test_ldb_modify_during_search(state, false, true);
+}
+
static int ldb_case_test_setup(void **state)
{
int ret;
@@ -2059,6 +2396,18 @@ int main(int argc, const char **argv)
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_modify_during_unindexed_search,
+ ldb_search_test_setup,
+ ldb_search_test_teardown),
+ cmocka_unit_test_setup_teardown(test_ldb_modify_during_indexed_search,
+ ldb_search_test_setup,
+ ldb_search_test_teardown),
+ cmocka_unit_test_setup_teardown(test_ldb_rename_during_unindexed_search,
+ ldb_search_test_setup,
+ ldb_search_test_teardown),
+ cmocka_unit_test_setup_teardown(test_ldb_rename_during_indexed_search,
+ 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),
--
2.9.4
From 63e6921b57b48189fa43079bfa14579a8c1afe6f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 22 May 2017 16:18:20 +1200
Subject: [PATCH 09/35] ldb:tests: Add test encoding current locking behaviour
during ldb_search()
Currently, a lock is not held against modifications once the final
record is returned via a callback, so modifications can be made
during the DONE callback. This makes it hard to write modules
that interpert an ldb search result and do further processing
so will change in the future to allow the full search to be
atomic.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/tests/ldb_mod_op_test.c | 238 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 238 insertions(+)
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 475138e..2840357 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -1847,6 +1847,241 @@ static void test_ldb_rename_during_unindexed_search(void **state)
return test_ldb_modify_during_search(state, false, true);
}
+/*
+ * This test is also complex.
+ *
+ * The purpose is to test if a modify can occur during an ldb_search()
+ * before the end of the callback
+ *
+ * This would be a failure if if in process
+ * (1) and (2):
+ * - (1) ldb_search() starts and calls back for a number of entries
+ * - (2) an entry in the DB is allowed to change before the callback returns
+ * - (1) the callback can see the modification
+ *
+ */
+
+/*
+ * This purpose of this callback is to trigger a write in
+ * the child process while a search DONE callback is in progress.
+ *
+ * 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_during_whole_search_callback1(struct ldb_request *req,
+ struct ldb_reply *ares)
+{
+ int ret;
+ int pipes[2];
+ char buf[2];
+ struct modify_during_search_test_ctx *ctx = req->context;
+ struct ldb_dn *search_dn;
+ struct ldb_result *res2;
+ unsigned res_count;
+ switch (ares->type) {
+ case LDB_REPLY_ENTRY:
+ case LDB_REPLY_REFERRAL:
+ return LDB_SUCCESS;
+
+ case LDB_REPLY_DONE:
+ break;
+ }
+
+ 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;
+ struct ldb_message_element *el;
+ 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,
+ "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(ctx->test_ctx->ldb);
+ if (ret != 0) {
+ exit(ret);
+ }
+
+ if (write(pipes[1], "GO", 2) != 2) {
+ exit(LDB_ERR_OPERATIONS_ERROR);
+ }
+
+ ret = ldb_modify(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);
+
+ sleep(3);
+
+ /*
+ * If writes are not blocked until after this function, we
+ * will be able to successfully search for this modification
+ * here
+ */
+
+ search_dn = ldb_dn_new_fmt(ares, ctx->test_ctx->ldb,
+ "cn=test_search_cn,"
+ "dc=search_test_entry");
+
+ ret = ldb_search(ctx->test_ctx->ldb, ares,
+ &res2, search_dn, LDB_SCOPE_BASE, NULL,
+ "filterAttr=TRUE");
+
+ /*
+ * We do this in an unusual order, because if we fail an assert before
+ * ldb_request_done(), we will also fail to clean up as we hold locks.
+ */
+
+ res_count = res2->count;
+ ldb_request_done(req, LDB_SUCCESS);
+ assert_int_equal(ret, 0);
+
+ /*
+ * We got the result because of this
+ * tests wants to demonstrate.
+ *
+ * Once the bug is fixed, it should
+ * change to assert_int_equal(res_count, 0);
+ */
+ assert_int_equal(res_count, 1);
+
+ return ret;
+}
+
+static void test_ldb_modify_during_whole_search(void **state)
+{
+ struct search_test_ctx *search_test_ctx = talloc_get_type_abort(*state,
+ struct search_test_ctx);
+ struct modify_during_search_test_ctx
+ ctx =
+ {
+ .test_ctx = search_test_ctx->ldb_test_ctx,
+ };
+
+ int ret;
+ struct ldb_request *req;
+ pid_t pid;
+ int wstatus;
+ struct ldb_dn *search_dn;
+ struct ldb_result *res2;
+
+ tevent_loop_allow_nesting(search_test_ctx->ldb_test_ctx->ev);
+
+ 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);
+
+
+ /*
+ * 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,
+ ctx.basedn,
+ LDB_SCOPE_SUBTREE,
+ "(&(!(filterAttr=*))"
+ "(cn=test_search_cn))",
+ NULL,
+ NULL,
+ &ctx,
+ test_ldb_modify_during_whole_search_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);
+
+ pid = waitpid(ctx.child_pid, &wstatus, 0);
+ assert_int_equal(pid, ctx.child_pid);
+
+ assert_true(WIFEXITED(wstatus));
+
+ assert_int_equal(WEXITSTATUS(wstatus), 0);
+
+ /*
+ * If writes are blocked until after the search function, 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;
@@ -2408,6 +2643,9 @@ int main(int argc, const char **argv)
cmocka_unit_test_setup_teardown(test_ldb_rename_during_indexed_search,
ldb_search_test_setup,
ldb_search_test_teardown),
+ 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_attrs_case_insensitive,
ldb_case_test_setup,
ldb_case_test_teardown),
--
2.9.4
From 38743b2a12e774056d6d6932bbfe4a35604a0ec7 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Jun 2017 12:10:51 +1200
Subject: [PATCH 10/35] ldb: Add read_lock and read_unlock to ldb_module_ops
This will be used to implement read locking in ldb_tdb
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/ABI/ldb-1.1.31.sigs | 2 ++
lib/ldb/common/ldb_modules.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
lib/ldb/include/ldb_module.h | 4 ++++
3 files changed, 52 insertions(+)
diff --git a/lib/ldb/ABI/ldb-1.1.31.sigs b/lib/ldb/ABI/ldb-1.1.31.sigs
index d183708..1be2ae7 100644
--- a/lib/ldb/ABI/ldb-1.1.31.sigs
+++ b/lib/ldb/ABI/ldb-1.1.31.sigs
@@ -186,6 +186,8 @@ ldb_next_del_trans: int (struct ldb_module *)
ldb_next_end_trans: int (struct ldb_module *)
ldb_next_init: int (struct ldb_module *)
ldb_next_prepare_commit: int (struct ldb_module *)
+ldb_next_read_lock: int (struct ldb_module *)
+ldb_next_read_unlock: int (struct ldb_module *)
ldb_next_remote_request: int (struct ldb_module *, struct ldb_request *)
ldb_next_request: int (struct ldb_module *, struct ldb_request *)
ldb_next_start_trans: int (struct ldb_module *)
diff --git a/lib/ldb/common/ldb_modules.c b/lib/ldb/common/ldb_modules.c
index ca93299..3dd0438 100644
--- a/lib/ldb/common/ldb_modules.c
+++ b/lib/ldb/common/ldb_modules.c
@@ -641,6 +641,52 @@ int ldb_next_end_trans(struct ldb_module *module)
return ret;
}
+int ldb_next_read_lock(struct ldb_module *module)
+{
+ int ret;
+ FIND_OP(module, read_lock);
+ ret = module->ops->read_lock(module);
+ if (ret == LDB_SUCCESS) {
+ return ret;
+ }
+ if (!ldb_errstring(module->ldb)) {
+ /* Set a default error string, to place the blame somewhere */
+ ldb_asprintf_errstring(module->ldb,
+ "read_lock error in module %s: %s (%d)",
+ module->ops->name, ldb_strerror(ret),
+ ret);
+ }
+ if ((module && module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
+ ldb_debug(module->ldb, LDB_DEBUG_TRACE,
+ "ldb_next_read_lock error: %s",
+ ldb_errstring(module->ldb));
+ }
+ return ret;
+}
+
+int ldb_next_read_unlock(struct ldb_module *module)
+{
+ int ret;
+ FIND_OP(module, read_unlock);
+ ret = module->ops->read_unlock(module);
+ if (ret == LDB_SUCCESS) {
+ return ret;
+ }
+ if (!ldb_errstring(module->ldb)) {
+ /* Set a default error string, to place the blame somewhere */
+ ldb_asprintf_errstring(module->ldb,
+ "read_unlock error in module %s: %s (%d)",
+ module->ops->name, ldb_strerror(ret),
+ ret);
+ }
+ if ((module && module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
+ ldb_debug(module->ldb, LDB_DEBUG_TRACE,
+ "ldb_next_read_unlock error: %s",
+ ldb_errstring(module->ldb));
+ }
+ return ret;
+}
+
int ldb_next_prepare_commit(struct ldb_module *module)
{
int ret;
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 3d56e68..0f5feeb 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -77,6 +77,8 @@ struct ldb_module_ops {
int (*end_transaction)(struct ldb_module *);
int (*del_transaction)(struct ldb_module *);
int (*sequence_number)(struct ldb_module *, struct ldb_request *);
+ int (*read_lock)(struct ldb_module *);
+ int (*read_unlock)(struct ldb_module *);
void *private_data;
};
@@ -203,6 +205,8 @@ int ldb_next_end_trans(struct ldb_module *module);
int ldb_next_del_trans(struct ldb_module *module);
int ldb_next_prepare_commit(struct ldb_module *module);
int ldb_next_init(struct ldb_module *module);
+int ldb_next_read_lock(struct ldb_module *module);
+int ldb_next_read_unlock(struct ldb_module *module);
void ldb_set_errstring(struct ldb_context *ldb, const char *err_string);
void ldb_asprintf_errstring(struct ldb_context *ldb, const char *format, ...) PRINTF_ATTRIBUTE(2,3);
--
2.9.4
From e6f138055125aa33acdf2643f35b11b1167602b5 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 12 May 2017 01:39:08 +0200
Subject: [PATCH 11/35] ldb_tdb: Implement read_lock and read_unlock module
operations
This allows Samba to provide a consistent view of the DB
despite the use of multiple databases via the partitions module
and over multiple callbacks via a module stack.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/ldb_tdb/ldb_tdb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index ad15a5e..2ac1967 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -1577,6 +1577,8 @@ static const struct ldb_module_ops ltdb_ops = {
.end_transaction = ltdb_end_trans,
.prepare_commit = ltdb_prepare_commit,
.del_transaction = ltdb_del_trans,
+ .read_lock = ltdb_lock_read,
+ .read_unlock = ltdb_unlock_read,
};
/*
--
2.9.4
From 1e564a86329f34e5e56162ef83431e436a47bdde Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Jun 2017 13:56:46 +1200
Subject: [PATCH 12/35] ldb: Lock the whole backend database for the duration
of a search
We must hold locks not just for the duration of each search, but for the whole search
as our module stack may make multiple search requests to build up the whole result.
This is explains a number of replication and read corruption issues in Samba
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/common/ldb.c | 159 +++++++++++++++++++++++++++++++++++++++-
lib/ldb/tests/ldb_mod_op_test.c | 10 +--
2 files changed, 160 insertions(+), 9 deletions(-)
diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index 700d89c..a4d9977 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -966,6 +966,146 @@ static int ldb_msg_check_element_flags(struct ldb_context *ldb,
return LDB_SUCCESS;
}
+/*
+ * This context allows us to make the unlock be a talloc destructor
+ *
+ * This ensures that a request started, but not waited on, will still
+ * unlock.
+ */
+struct ldb_db_lock_context {
+ struct ldb_request *req;
+ struct ldb_context *ldb;
+};
+
+/*
+ * We have to have a the unlock on a destructor so that we unlock the
+ * DB if a caller calls talloc_free(req). We trust that the ldb
+ * context has not already gone away.
+ */
+static int ldb_db_lock_destructor(struct ldb_db_lock_context *lock_context)
+{
+ int ret;
+ struct ldb_module *next_module;
+ FIRST_OP_NOERR(lock_context->ldb, read_unlock);
+ if (next_module != NULL) {
+ ret = next_module->ops->read_unlock(next_module);
+ } else {
+ ret = LDB_SUCCESS;
+ }
+
+ if (ret != LDB_SUCCESS) {
+ ldb_debug(lock_context->ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to unlock db: %s / %s",
+ ldb_errstring(lock_context->ldb),
+ ldb_strerror(ret));
+ }
+ return 0;
+}
+
+static int ldb_lock_backend_callback(struct ldb_request *req,
+ struct ldb_reply *ares)
+{
+ struct ldb_db_lock_context *lock_context;
+ int ret;
+
+ lock_context = talloc_get_type(req->context,
+ struct ldb_db_lock_context);
+
+ if (!ares) {
+ return ldb_module_done(lock_context->req, NULL, NULL,
+ LDB_ERR_OPERATIONS_ERROR);
+ }
+ if (ares->error != LDB_SUCCESS || ares->type == LDB_REPLY_DONE) {
+ ret = ldb_module_done(lock_context->req, ares->controls,
+ ares->response, ares->error);
+ /*
+ * If this is a LDB_REPLY_DONE or an error, unlock the
+ * DB by calling the destructor on this context
+ */
+ talloc_free(lock_context);
+ return ret;
+ }
+
+ /* Otherwise pass on the callback */
+ switch (ares->type) {
+ case LDB_REPLY_ENTRY:
+ return ldb_module_send_entry(lock_context->req, ares->message,
+ ares->controls);
+
+ case LDB_REPLY_REFERRAL:
+ return ldb_module_send_referral(lock_context->req,
+ ares->referral);
+ default:
+ /* Can't happen */
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+}
+
+/*
+ * Do an ldb_search() with a lock held, but release it if the request
+ * is freed with talloc_free()
+ */
+static int lock_search(struct ldb_module *lock_module, struct ldb_request *req)
+{
+ /* Used in FIRST_OP_NOERR to find where to send the lock request */
+ struct ldb_module *next_module = NULL;
+ struct ldb_request *down_req = NULL;
+ struct ldb_db_lock_context *lock_context;
+ struct ldb_context *ldb = ldb_module_get_ctx(lock_module);
+ int ret;
+
+ lock_context = talloc(req, struct ldb_db_lock_context);
+ if (lock_context == NULL) {
+ return ldb_oom(ldb);
+ }
+
+ lock_context->ldb = ldb;
+ lock_context->req = req;
+
+ ret = ldb_build_search_req_ex(&down_req, ldb, req,
+ req->op.search.base,
+ req->op.search.scope,
+ req->op.search.tree,
+ req->op.search.attrs,
+ req->controls,
+ lock_context,
+ ldb_lock_backend_callback,
+ req);
+ LDB_REQ_SET_LOCATION(down_req);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ /* call DB lock */
+ FIRST_OP_NOERR(ldb, read_lock);
+ if (next_module != NULL) {
+ ret = next_module->ops->read_lock(next_module);
+ } else {
+ ret = LDB_ERR_UNSUPPORTED_CRITICAL_EXTENSION;
+ }
+
+ if (ret == LDB_ERR_UNSUPPORTED_CRITICAL_EXTENSION) {
+ /* We might be talking LDAP */
+ ldb_reset_err_string(ldb);
+ ret = 0;
+ TALLOC_FREE(lock_context);
+
+ return ldb_next_request(lock_module, req);
+ } else if ((ret != LDB_SUCCESS) && (ldb->err_string == NULL)) {
+ /* if no error string was setup by the backend */
+ ldb_asprintf_errstring(ldb, "Failed to get DB lock: %s (%d)",
+ ldb_strerror(ret), ret);
+ } else {
+ talloc_set_destructor(lock_context, ldb_db_lock_destructor);
+ }
+
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ return ldb_next_request(lock_module, down_req);
+}
/*
start an ldb request
@@ -991,15 +1131,32 @@ int ldb_request(struct ldb_context *ldb, struct ldb_request *req)
/* call the first module in the chain */
switch (req->operation) {
case LDB_SEARCH:
+ {
+ /*
+ * A fake module to allow ldb_next_request() to be
+ * re-used and to keep the locking out of this function.
+ */
+ static const struct ldb_module_ops lock_module_ops = {
+ .name = "lock_searches",
+ .search = lock_search
+ };
+ struct ldb_module lock_module = {
+ .ldb = ldb,
+ .next = ldb->modules,
+ .ops = &lock_module_ops
+ };
+ next_module = &lock_module;
+
/* due to "ldb_build_search_req" base DN always != NULL */
if (!ldb_dn_validate(req->op.search.base)) {
ldb_asprintf_errstring(ldb, "ldb_search: invalid basedn '%s'",
ldb_dn_get_linearized(req->op.search.base));
return LDB_ERR_INVALID_DN_SYNTAX;
}
- FIRST_OP(ldb, search);
+
ret = next_module->ops->search(next_module, req);
break;
+ }
case LDB_ADD:
if (!ldb_dn_validate(req->op.add.message->dn)) {
ldb_asprintf_errstring(ldb, "ldb_add: invalid dn '%s'",
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 2840357..6357f83 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -1991,14 +1991,8 @@ static int test_ldb_modify_during_whole_search_callback1(struct ldb_request *req
ldb_request_done(req, LDB_SUCCESS);
assert_int_equal(ret, 0);
- /*
- * We got the result because of this
- * tests wants to demonstrate.
- *
- * Once the bug is fixed, it should
- * change to assert_int_equal(res_count, 0);
- */
- assert_int_equal(res_count, 1);
+ /* We should not have got the result */
+ assert_int_equal(res_count, 0);
return ret;
}
--
2.9.4
From c7396a6ec9ce27cf2eb7fc1e246e7932d66d9a47 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 16 Jun 2017 12:18:39 +1200
Subject: [PATCH 13/35] ldb:tests: Correct comment about version numbers
(ldb releases have been made while this patch set was in train)
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/tests/ldb_mod_op_test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 6357f83..68b7c79 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -1285,7 +1285,7 @@ static void test_search_match_basedn(void **state)
* - (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
+ * With ldb 1.1.31 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()
*/
@@ -1327,7 +1327,7 @@ static int test_ldb_search_against_transaction_callback2(struct ldb_request *req
* 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
+ * however in ldb 1.1.31 ltdb_search() forgets to take the all-record
* lock (except the very first time) due to a ref-counting bug.
*
*/
--
2.9.4
From edfe48ceadfb635c8ba59b3329744b861d12a197 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 16 Jun 2017 12:19:00 +1200
Subject: [PATCH 14/35] ldb:tests: Add test to show that locks are released on
TALLOC_FREE(req)
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/tests/ldb_mod_op_test.c | 226 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 226 insertions(+)
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 68b7c79..96d2dd2 100644
--- 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),
--
2.9.4
From aefdec5fdd5a0e461538dffebcaa11a0a1271a9b Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 16 Jun 2017 15:44:46 +1200
Subject: [PATCH 15/35] ldb:tests: Extend api.py testsuite to show transaction
contents can not be seen outside the transaction
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/tests/python/api.py | 66 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py
index 4d290ef..f9999ba 100755
--- a/lib/ldb/tests/python/api.py
+++ b/lib/ldb/tests/python/api.py
@@ -1315,6 +1315,72 @@ class LdbResultTests(TestCase):
found = True
self.assertTrue(found)
+
+ # Show that search results can't see into a transaction
+ def test_search_against_trans(self):
+ found11 = False
+
+ (r1, w1) = os.pipe()
+
+ (r2, w2) = os.pipe()
+
+ # For the first element, fork a child that will
+ # write to the DB
+ pid = os.fork()
+ if pid == 0:
+ # In the child, re-open
+ del(self.l)
+ gc.collect()
+
+ child_ldb = ldb.Ldb(self.filename)
+ # start a transaction
+ child_ldb.transaction_start()
+
+ # write to it
+ child_ldb.add({"dn": "OU=OU11,DC=SAMBA,DC=ORG",
+ "name": b"samba.org"})
+
+ os.write(w1, b"added")
+
+ # Now wait for the search to be done
+ os.read(r2, 6)
+
+ # and commit
+ try:
+ child_ldb.transaction_commit()
+ except LdbError as err:
+ # We print this here to see what went wrong in the child
+ print(err)
+ os._exit(1)
+
+ os.write(w1, b"transaction")
+ os._exit(0)
+
+ self.assertEqual(os.read(r1, 5), b"added")
+
+ # This should not turn up until the transaction is concluded
+ res11 = self.l.search(base="OU=OU11,DC=SAMBA,DC=ORG",
+ scope=ldb.SCOPE_BASE)
+ self.assertEqual(len(res11), 0)
+
+ os.write(w2, b"search")
+
+ # Now wait for the transaction to be done. This should
+ # deadlock, but the search doesn't hold a read lock for the
+ # iterator lifetime currently.
+ self.assertEqual(os.read(r1, 11), b"transaction")
+
+ # This should now turn up, as the transaction is over
+ res11 = self.l.search(base="OU=OU11,DC=SAMBA,DC=ORG",
+ scope=ldb.SCOPE_BASE)
+ self.assertEqual(len(res11), 1)
+
+ self.assertFalse(found11)
+
+ (got_pid, status) = os.waitpid(pid, 0)
+ self.assertEqual(got_pid, pid)
+
+
def test_search_iter_against_trans(self):
found = False
found11 = False
--
2.9.4
From 51dd6106ce86e010d335ce8e5f71dce2cf6ce380 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 16 Jun 2017 15:49:16 +1200
Subject: [PATCH 16/35] ldb:tests: Extend api.py testsuite to show
transaction_commit() blocks against the whole-db read lock
The new ldb whole-db lock behaviour now allows this test
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/tests/python/api.py | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py
index f9999ba..0c3b20e 100755
--- a/lib/ldb/tests/python/api.py
+++ b/lib/ldb/tests/python/api.py
@@ -1393,16 +1393,13 @@ class LdbResultTests(TestCase):
(r2, w2) = os.pipe()
- l = next(res)
- if str(l.dn) == "OU=OU10,DC=SAMBA,DC=ORG":
- found = True
-
# For the first element, with the sequence open (which
# means with ldb locks held), fork a child that will
# write to the DB
pid = os.fork()
if pid == 0:
# In the child, re-open
+ del(res)
del(self.l)
gc.collect()
@@ -1439,25 +1436,26 @@ class LdbResultTests(TestCase):
os.write(w2, b"search")
- # Now wait for the transaction to be done. This should
- # deadlock, but the search doesn't hold a read lock for the
- # iterator lifetime currently.
- self.assertEqual(os.read(r1, 11), b"transaction")
+ # allow the transaction to start
+ time.sleep(1)
# This should not turn up until the search finishes and
# removed the read lock, but for ldb_tdb that happened as soon
# as we called the first res.next()
res11 = self.l.search(base="OU=OU11,DC=SAMBA,DC=ORG",
scope=ldb.SCOPE_BASE)
- self.assertEqual(len(res11), 1)
+ self.assertEqual(len(res11), 0)
- # These results were actually collected at the first next(res) call
+ # These results are all collected at the first next(res) call
for l in res:
if str(l.dn) == "OU=OU10,DC=SAMBA,DC=ORG":
found = True
if str(l.dn) == "OU=OU11,DC=SAMBA,DC=ORG":
found11 = True
+ # Now wait for the transaction to be done.
+ self.assertEqual(os.read(r1, 11), b"transaction")
+
# This should now turn up, as the transaction is over and all
# read locks are gone
res11 = self.l.search(base="OU=OU11,DC=SAMBA,DC=ORG",
--
2.9.4
From 1c00357f2a1e82617d6d1588bfc8dffcf7a89167 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 23 Jun 2017 10:50:54 +0200
Subject: [PATCH 17/35] ldb:wscript: provide LDB_VERSION_{MAJOR,MINOR,RELEASE}
in ldb_version.h
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/wscript | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index 1fce3b3..d827775 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -205,8 +205,24 @@ def build(bld):
abi_match = abi_match)
# generate a include/ldb_version.h
+ def generate_ldb_version_h(t):
+ '''generate a vscript file for our public libraries'''
+
+ tgt = t.outputs[0].bldpath(t.env)
+
+ v = t.env.LDB_VERSION.split('.')
+
+ f = open(tgt, mode='w')
+ try:
+ f.write('#define LDB_VERSION "%s"\n' % t.env.LDB_VERSION)
+ f.write('#define LDB_VERSION_MAJOR %d\n' % int(v[0]))
+ f.write('#define LDB_VERSION_MINOR %d\n' % int(v[1]))
+ f.write('#define LDB_VERSION_RELEASE %d\n' % int(v[2]))
+ finally:
+ f.close()
+ return
t = bld.SAMBA_GENERATOR('ldb_version.h',
- rule='echo "#define LDB_VERSION \\"${LDB_VERSION}\\"" > ${TGT}',
+ rule=generate_ldb_version_h,
dep_vars=['LDB_VERSION'],
target='include/ldb_version.h',
public_headers='include/ldb_version.h',
--
2.9.4
From 38af7017b45961eea10f9d1b1f1b67fc3a4dcc4c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 30 Jun 2017 08:09:38 +0200
Subject: [PATCH 18/35] ldb:wscript: define
EXPECTED_SYSTEM_LDB_VERSION_{MAJOR,MINOR,RELEASE}
This indicates what feature set Samba assumes from the used
libldb from the system.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12859
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/wscript | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index d827775..2e77f00 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -91,6 +91,12 @@ def configure(conf):
implied_deps='replace talloc tdb tevent'):
conf.define('USING_SYSTEM_LDB', 1)
+ if conf.CONFIG_SET('USING_SYSTEM_LDB'):
+ v = VERSION.split('.')
+ conf.DEFINE('EXPECTED_SYSTEM_LDB_VERSION_MAJOR', int(v[0]))
+ conf.DEFINE('EXPECTED_SYSTEM_LDB_VERSION_MINOR', int(v[1]))
+ conf.DEFINE('EXPECTED_SYSTEM_LDB_VERSION_RELEASE', int(v[2]))
+
if conf.env.standalone_ldb:
conf.CHECK_XSLTPROC_MANPAGES()
--
2.9.4
From da4aaaf701bb0262eb02cd4f31d709f71d815611 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 30 Jun 2017 08:14:02 +0200
Subject: [PATCH 19/35] ldb:includes: protect ldb_modules.h from being used by
Samba < 4.7
Samba versions before 4.7 are incompatible with the read_[un]lock()
behaviour introduced into ldb.
This makes sure older Samba versions fail to compile against
ldb >= 1.2.0.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12859
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
lib/ldb/include/ldb_module.h | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 0f5feeb..8ad212a 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -35,6 +35,41 @@
#include <ldb.h>
+#if defined(_SAMBA_BUILD_) && defined(USING_SYSTEM_LDB)
+
+/*
+ * Versions before 1.2.0 doesn't define these values
+ * so we assime 1.1.29 (which was used in Samba 4.6)
+ *
+ * See https://bugzilla.samba.org/show_bug.cgi?id=12859
+ */
+#ifndef EXPECTED_SYSTEM_LDB_VERSION_MAJOR
+#define EXPECTED_SYSTEM_LDB_VERSION_MAJOR 1
+#endif
+#ifndef EXPECTED_SYSTEM_LDB_VERSION_MINOR
+#define EXPECTED_SYSTEM_LDB_VERSION_MINOR 1
+#endif
+#ifndef EXPECTED_SYSTEM_LDB_VERSION_MINOR
+#define EXPECTED_SYSTEM_LDB_VERSION_MINOR 29
+#endif
+
+/*
+ * Only Samba versions which expect ldb >= 1.2.0
+ * are compatible with read_[un]lock() behaviour.
+ *
+ * See https://bugzilla.samba.org/show_bug.cgi?id=12859
+ */
+#if EXPECTED_SYSTEM_LDB_VERSION_MAJOR > 1
+#define __LDB_READ_LOCK_COMPATIBLE__ 1
+#elif EXPECTED_SYSTEM_LDB_VERSION_MINOR > 1
+#define __LDB_READ_LOCK_COMPATIBLE__ 1
+#endif
+#ifndef __LDB_READ_LOCK_COMPATIBLE__
+#error "Samba < 4.7 is not compatible with this version of ldb due to assumptions around read locks"
+#endif
+
+#endif /* defined(_SAMBA_BUILD_) && defined(USING_SYSTEM_LDB) */
+
struct ldb_context;
struct ldb_module;
--
2.9.4
From 0d10d50b419a2cf6876efee5a6052223f70d2afc Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 27 Apr 2017 08:18:38 +1200
Subject: [PATCH 20/35] ldb_tdb: Use TDB_DISALLOW_NESTING
The top level LDB code and the ldb_tdb code already keeps a reference
count of so-called nested transactions so do not allow tdb to do also.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb/ldb_tdb/ldb_tdb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 2ac1967..1176dd5 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -1612,7 +1612,7 @@ static int ltdb_connect(struct ldb_context *ldb, const char *url,
path = url;
}
- tdb_flags = TDB_DEFAULT | TDB_SEQNUM;
+ tdb_flags = TDB_DEFAULT | TDB_SEQNUM | TDB_DISALLOW_NESTING;
/* check for the 'nosync' option */
if (flags & LDB_FLG_NOSYNC) {
--
2.9.4
From b314ce5e25c23cfde8c062e82cf84b6d31ea9af9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 27 Apr 2017 08:49:42 +1200
Subject: [PATCH 21/35] ldb_tdb: use tdb_transaction_active() to determine if
we are in a transaction
This avoids using the ldb_tdb level reference count
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb/ldb_tdb/ldb_search.c | 2 +-
lib/ldb/ldb_tdb/ldb_tdb.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/ldb/ldb_tdb/ldb_search.c b/lib/ldb/ldb_tdb/ldb_search.c
index 53355e0..d33fe40 100644
--- a/lib/ldb/ldb_tdb/ldb_search.c
+++ b/lib/ldb/ldb_tdb/ldb_search.c
@@ -489,7 +489,7 @@ static int ltdb_search_full(struct ltdb_context *ctx)
int ret;
ctx->error = LDB_SUCCESS;
- if (ltdb->in_transaction != 0) {
+ if (tdb_transaction_active(ltdb->tdb)) {
ret = tdb_traverse(ltdb->tdb, search_func, ctx);
} else {
ret = tdb_traverse_read(ltdb->tdb, search_func, ctx);
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 1176dd5..c1156ca 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -100,7 +100,7 @@ int ltdb_lock_read(struct ldb_module *module)
struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
int ret = 0;
- if (ltdb->in_transaction == 0 &&
+ if (tdb_transaction_active(ltdb->tdb) == false &&
ltdb->read_lock_count == 0) {
ret = tdb_lockall_read(ltdb->tdb);
}
@@ -117,7 +117,8 @@ int ltdb_unlock_read(struct ldb_module *module)
{
void *data = ldb_module_get_private(module);
struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
- if (ltdb->in_transaction == 0 && ltdb->read_lock_count == 1) {
+ if (tdb_transaction_active(ltdb->tdb) == false
+ && ltdb->read_lock_count == 1) {
tdb_unlockall_read(ltdb->tdb);
ltdb->read_lock_count--;
return 0;
@@ -223,7 +224,7 @@ static int ltdb_modified(struct ldb_module *module, struct ldb_dn *dn)
/* only allow modifies inside a transaction, otherwise the
* ldb is unsafe */
- if (ltdb->in_transaction == 0) {
+ if (tdb_transaction_active(ltdb->tdb) == false) {
ldb_set_errstring(ldb_module_get_ctx(module), "ltdb modify without transaction");
return LDB_ERR_OPERATIONS_ERROR;
}
--
2.9.4
From fdd6056b95fcd483513f1d74db6161d79add2fca Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 20:20:35 +1000
Subject: [PATCH 22/35] ldb: Remove in_transaction from ldb_tdb
We no longer need to track this, the tdb_transaction_start() call
prevents us from seeing nested transactions.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb/ldb_tdb/ldb_tdb.c | 14 ++++++--------
lib/ldb/ldb_tdb/ldb_tdb.h | 2 --
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index c1156ca..5bfa0a1 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -1148,8 +1148,6 @@ static int ltdb_start_trans(struct ldb_module *module)
return ltdb_err_map(tdb_error(ltdb->tdb));
}
- ltdb->in_transaction++;
-
ltdb_index_transaction_start(module);
return LDB_SUCCESS;
@@ -1161,20 +1159,22 @@ static int ltdb_prepare_commit(struct ldb_module *module)
void *data = ldb_module_get_private(module);
struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
- if (ltdb->in_transaction != 1) {
- return LDB_SUCCESS;
+ if (tdb_transaction_active(ltdb->tdb) == false) {
+ ldb_set_errstring(ldb_module_get_ctx(module),
+ "ltdb_prepare_commit() called "
+ "without transaction active");
+
+ return LDB_ERR_OPERATIONS_ERROR;
}
ret = ltdb_index_transaction_commit(module);
if (ret != LDB_SUCCESS) {
tdb_transaction_cancel(ltdb->tdb);
- ltdb->in_transaction--;
return ret;
}
if (tdb_transaction_prepare_commit(ltdb->tdb) != 0) {
ret = ltdb_err_map(tdb_error(ltdb->tdb));
- ltdb->in_transaction--;
ldb_asprintf_errstring(ldb_module_get_ctx(module),
"Failure during tdb_transaction_prepare_commit(): %s -> %s",
tdb_errorstr(ltdb->tdb),
@@ -1200,7 +1200,6 @@ static int ltdb_end_trans(struct ldb_module *module)
}
}
- ltdb->in_transaction--;
ltdb->prepared_commit = false;
if (tdb_transaction_commit(ltdb->tdb) != 0) {
@@ -1220,7 +1219,6 @@ static int ltdb_del_trans(struct ldb_module *module)
void *data = ldb_module_get_private(module);
struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
- ltdb->in_transaction--;
if (ltdb_index_transaction_cancel(module) != 0) {
tdb_transaction_cancel(ltdb->tdb);
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.h b/lib/ldb/ldb_tdb/ldb_tdb.h
index 26ae68e..41dc50e 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.h
+++ b/lib/ldb/ldb_tdb/ldb_tdb.h
@@ -22,8 +22,6 @@ struct ltdb_private {
bool attribute_indexes;
} *cache;
- int in_transaction;
-
bool check_base;
bool disallow_dn_filter;
struct ltdb_idxptr *idxptr;
--
2.9.4
From bf6e5507167fd0996c6689abf418d45d2f0f36f1 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 11 Apr 2017 17:50:08 +0200
Subject: [PATCH 23/35] TODO: ldb: version 1.2.0
* handle one more LDB_FLAG_INTERNAL_DISABLE_SINGLE_VALUE_CHECK
case in ldb_tdb
* fix ldb_tdb locking (performance) problems
* fix ldb_tdb search inconsistencies by adding
read_[un]lock() hooks to the module stack
(bug #12858)
* add cmocka based tests for the locking issues
* ldb_version.h provides LDB_VERSION_{MAJOR,MINOR,RELEASE} defines
* use tdb_transaction_active() to track transaction state
Note: that this release (as well as 1.1.30 and 1.1.31)
may cause problems for older applications, e.g. Samba
See https://bugzilla.samba.org/show_bug.cgi?id=12859
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb/ABI/ldb-1.1.31.sigs | 2 -
lib/ldb/ABI/ldb-1.2.0.sigs | 276 ++++++++++++++++++++++++++++++++++
lib/ldb/ABI/pyldb-util-1.2.0.sigs | 2 +
lib/ldb/ABI/pyldb-util.py3-1.2.0.sigs | 2 +
lib/ldb/wscript | 2 +-
5 files changed, 281 insertions(+), 3 deletions(-)
create mode 100644 lib/ldb/ABI/ldb-1.2.0.sigs
create mode 100644 lib/ldb/ABI/pyldb-util-1.2.0.sigs
create mode 100644 lib/ldb/ABI/pyldb-util.py3-1.2.0.sigs
diff --git a/lib/ldb/ABI/ldb-1.1.31.sigs b/lib/ldb/ABI/ldb-1.1.31.sigs
index 1be2ae7..d183708 100644
--- a/lib/ldb/ABI/ldb-1.1.31.sigs
+++ b/lib/ldb/ABI/ldb-1.1.31.sigs
@@ -186,8 +186,6 @@ ldb_next_del_trans: int (struct ldb_module *)
ldb_next_end_trans: int (struct ldb_module *)
ldb_next_init: int (struct ldb_module *)
ldb_next_prepare_commit: int (struct ldb_module *)
-ldb_next_read_lock: int (struct ldb_module *)
-ldb_next_read_unlock: int (struct ldb_module *)
ldb_next_remote_request: int (struct ldb_module *, struct ldb_request *)
ldb_next_request: int (struct ldb_module *, struct ldb_request *)
ldb_next_start_trans: int (struct ldb_module *)
diff --git a/lib/ldb/ABI/ldb-1.2.0.sigs b/lib/ldb/ABI/ldb-1.2.0.sigs
new file mode 100644
index 0000000..1be2ae7
--- /dev/null
+++ b/lib/ldb/ABI/ldb-1.2.0.sigs
@@ -0,0 +1,276 @@
+ldb_add: int (struct ldb_context *, const struct ldb_message *)
+ldb_any_comparison: int (struct ldb_context *, void *, ldb_attr_handler_t, const struct ldb_val *, const struct ldb_val *)
+ldb_asprintf_errstring: void (struct ldb_context *, const char *, ...)
+ldb_attr_casefold: char *(TALLOC_CTX *, const char *)
+ldb_attr_dn: int (const char *)
+ldb_attr_in_list: int (const char * const *, const char *)
+ldb_attr_list_copy: const char **(TALLOC_CTX *, const char * const *)
+ldb_attr_list_copy_add: const char **(TALLOC_CTX *, const char * const *, const char *)
+ldb_base64_decode: int (char *)
+ldb_base64_encode: char *(TALLOC_CTX *, const char *, int)
+ldb_binary_decode: struct ldb_val (TALLOC_CTX *, const char *)
+ldb_binary_encode: char *(TALLOC_CTX *, struct ldb_val)
+ldb_binary_encode_string: char *(TALLOC_CTX *, const char *)
+ldb_build_add_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_del_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_extended_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const char *, void *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_mod_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_rename_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, struct ldb_dn *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_search_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, enum ldb_scope, const char *, const char * const *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_search_req_ex: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, enum ldb_scope, struct ldb_parse_tree *, const char * const *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_casefold: char *(struct ldb_context *, TALLOC_CTX *, const char *, size_t)
+ldb_casefold_default: char *(void *, TALLOC_CTX *, const char *, size_t)
+ldb_check_critical_controls: int (struct ldb_control **)
+ldb_comparison_binary: int (struct ldb_context *, void *, const struct ldb_val *, const struct ldb_val *)
+ldb_comparison_fold: int (struct ldb_context *, void *, const struct ldb_val *, const struct ldb_val *)
+ldb_connect: int (struct ldb_context *, const char *, unsigned int, const char **)
+ldb_control_to_string: char *(TALLOC_CTX *, const struct ldb_control *)
+ldb_controls_except_specified: struct ldb_control **(struct ldb_control **, TALLOC_CTX *, struct ldb_control *)
+ldb_debug: void (struct ldb_context *, enum ldb_debug_level, const char *, ...)
+ldb_debug_add: void (struct ldb_context *, const char *, ...)
+ldb_debug_end: void (struct ldb_context *, enum ldb_debug_level)
+ldb_debug_set: void (struct ldb_context *, enum ldb_debug_level, const char *, ...)
+ldb_delete: int (struct ldb_context *, struct ldb_dn *)
+ldb_dn_add_base: bool (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_add_base_fmt: bool (struct ldb_dn *, const char *, ...)
+ldb_dn_add_child: bool (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_add_child_fmt: bool (struct ldb_dn *, const char *, ...)
+ldb_dn_alloc_casefold: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_alloc_linearized: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_canonical_ex_string: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_canonical_string: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_check_local: bool (struct ldb_module *, struct ldb_dn *)
+ldb_dn_check_special: bool (struct ldb_dn *, const char *)
+ldb_dn_compare: int (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_compare_base: int (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_copy: struct ldb_dn *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_escape_value: char *(TALLOC_CTX *, struct ldb_val)
+ldb_dn_extended_add_syntax: int (struct ldb_context *, unsigned int, const struct ldb_dn_extended_syntax *)
+ldb_dn_extended_filter: void (struct ldb_dn *, const char * const *)
+ldb_dn_extended_syntax_by_name: const struct ldb_dn_extended_syntax *(struct ldb_context *, const char *)
+ldb_dn_from_ldb_val: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const struct ldb_val *)
+ldb_dn_get_casefold: const char *(struct ldb_dn *)
+ldb_dn_get_comp_num: int (struct ldb_dn *)
+ldb_dn_get_component_name: const char *(struct ldb_dn *, unsigned int)
+ldb_dn_get_component_val: const struct ldb_val *(struct ldb_dn *, unsigned int)
+ldb_dn_get_extended_comp_num: int (struct ldb_dn *)
+ldb_dn_get_extended_component: const struct ldb_val *(struct ldb_dn *, const char *)
+ldb_dn_get_extended_linearized: char *(TALLOC_CTX *, struct ldb_dn *, int)
+ldb_dn_get_ldb_context: struct ldb_context *(struct ldb_dn *)
+ldb_dn_get_linearized: const char *(struct ldb_dn *)
+ldb_dn_get_parent: struct ldb_dn *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_get_rdn_name: const char *(struct ldb_dn *)
+ldb_dn_get_rdn_val: const struct ldb_val *(struct ldb_dn *)
+ldb_dn_has_extended: bool (struct ldb_dn *)
+ldb_dn_is_null: bool (struct ldb_dn *)
+ldb_dn_is_special: bool (struct ldb_dn *)
+ldb_dn_is_valid: bool (struct ldb_dn *)
+ldb_dn_map_local: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *)
+ldb_dn_map_rebase_remote: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *)
+ldb_dn_map_remote: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *)
+ldb_dn_minimise: bool (struct ldb_dn *)
+ldb_dn_new: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const char *)
+ldb_dn_new_fmt: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const char *, ...)
+ldb_dn_remove_base_components: bool (struct ldb_dn *, unsigned int)
+ldb_dn_remove_child_components: bool (struct ldb_dn *, unsigned int)
+ldb_dn_remove_extended_components: void (struct ldb_dn *)
+ldb_dn_replace_components: bool (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_set_component: int (struct ldb_dn *, int, const char *, const struct ldb_val)
+ldb_dn_set_extended_component: int (struct ldb_dn *, const char *, const struct ldb_val *)
+ldb_dn_update_components: int (struct ldb_dn *, const struct ldb_dn *)
+ldb_dn_validate: bool (struct ldb_dn *)
+ldb_dump_results: void (struct ldb_context *, struct ldb_result *, FILE *)
+ldb_error_at: int (struct ldb_context *, int, const char *, const char *, int)
+ldb_errstring: const char *(struct ldb_context *)
+ldb_extended: int (struct ldb_context *, const char *, void *, struct ldb_result **)
+ldb_extended_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_filter_from_tree: char *(TALLOC_CTX *, const struct ldb_parse_tree *)
+ldb_get_config_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_get_create_perms: unsigned int (struct ldb_context *)
+ldb_get_default_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_get_event_context: struct tevent_context *(struct ldb_context *)
+ldb_get_flags: unsigned int (struct ldb_context *)
+ldb_get_opaque: void *(struct ldb_context *, const char *)
+ldb_get_root_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_get_schema_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_global_init: int (void)
+ldb_handle_get_event_context: struct tevent_context *(struct ldb_handle *)
+ldb_handle_new: struct ldb_handle *(TALLOC_CTX *, struct ldb_context *)
+ldb_handle_use_global_event_context: void (struct ldb_handle *)
+ldb_handler_copy: int (struct ldb_context *, void *, const struct ldb_val *, struct ldb_val *)
+ldb_handler_fold: int (struct ldb_context *, void *, const struct ldb_val *, struct ldb_val *)
+ldb_init: struct ldb_context *(TALLOC_CTX *, struct tevent_context *)
+ldb_ldif_message_string: char *(struct ldb_context *, TALLOC_CTX *, enum ldb_changetype, const struct ldb_message *)
+ldb_ldif_parse_modrdn: int (struct ldb_context *, const struct ldb_ldif *, TALLOC_CTX *, struct ldb_dn **, struct ldb_dn **, bool *, struct ldb_dn **, struct ldb_dn **)
+ldb_ldif_read: struct ldb_ldif *(struct ldb_context *, int (*)(void *), void *)
+ldb_ldif_read_file: struct ldb_ldif *(struct ldb_context *, FILE *)
+ldb_ldif_read_file_state: struct ldb_ldif *(struct ldb_context *, struct ldif_read_file_state *)
+ldb_ldif_read_free: void (struct ldb_context *, struct ldb_ldif *)
+ldb_ldif_read_string: struct ldb_ldif *(struct ldb_context *, const char **)
+ldb_ldif_write: int (struct ldb_context *, int (*)(void *, const char *, ...), void *, const struct ldb_ldif *)
+ldb_ldif_write_file: int (struct ldb_context *, FILE *, const struct ldb_ldif *)
+ldb_ldif_write_redacted_trace_string: char *(struct ldb_context *, TALLOC_CTX *, const struct ldb_ldif *)
+ldb_ldif_write_string: char *(struct ldb_context *, TALLOC_CTX *, const struct ldb_ldif *)
+ldb_load_modules: int (struct ldb_context *, const char **)
+ldb_map_add: int (struct ldb_module *, struct ldb_request *)
+ldb_map_delete: int (struct ldb_module *, struct ldb_request *)
+ldb_map_init: int (struct ldb_module *, const struct ldb_map_attribute *, const struct ldb_map_objectclass *, const char * const *, const char *, const char *)
+ldb_map_modify: int (struct ldb_module *, struct ldb_request *)
+ldb_map_rename: int (struct ldb_module *, struct ldb_request *)
+ldb_map_search: int (struct ldb_module *, struct ldb_request *)
+ldb_match_msg: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, struct ldb_dn *, enum ldb_scope)
+ldb_match_msg_error: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, struct ldb_dn *, enum ldb_scope, bool *)
+ldb_match_msg_objectclass: int (const struct ldb_message *, const char *)
+ldb_mod_register_control: int (struct ldb_module *, const char *)
+ldb_modify: int (struct ldb_context *, const struct ldb_message *)
+ldb_modify_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_module_call_chain: char *(struct ldb_request *, TALLOC_CTX *)
+ldb_module_connect_backend: int (struct ldb_context *, const char *, const char **, struct ldb_module **)
+ldb_module_done: int (struct ldb_request *, struct ldb_control **, struct ldb_extended *, int)
+ldb_module_flags: uint32_t (struct ldb_context *)
+ldb_module_get_ctx: struct ldb_context *(struct ldb_module *)
+ldb_module_get_name: const char *(struct ldb_module *)
+ldb_module_get_ops: const struct ldb_module_ops *(struct ldb_module *)
+ldb_module_get_private: void *(struct ldb_module *)
+ldb_module_init_chain: int (struct ldb_context *, struct ldb_module *)
+ldb_module_load_list: int (struct ldb_context *, const char **, struct ldb_module *, struct ldb_module **)
+ldb_module_new: struct ldb_module *(TALLOC_CTX *, struct ldb_context *, const char *, const struct ldb_module_ops *)
+ldb_module_next: struct ldb_module *(struct ldb_module *)
+ldb_module_popt_options: struct poptOption **(struct ldb_context *)
+ldb_module_send_entry: int (struct ldb_request *, struct ldb_message *, struct ldb_control **)
+ldb_module_send_referral: int (struct ldb_request *, char *)
+ldb_module_set_next: void (struct ldb_module *, struct ldb_module *)
+ldb_module_set_private: void (struct ldb_module *, void *)
+ldb_modules_hook: int (struct ldb_context *, enum ldb_module_hook_type)
+ldb_modules_list_from_string: const char **(struct ldb_context *, TALLOC_CTX *, const char *)
+ldb_modules_load: int (const char *, const char *)
+ldb_msg_add: int (struct ldb_message *, const struct ldb_message_element *, int)
+ldb_msg_add_empty: int (struct ldb_message *, const char *, int, struct ldb_message_element **)
+ldb_msg_add_fmt: int (struct ldb_message *, const char *, const char *, ...)
+ldb_msg_add_linearized_dn: int (struct ldb_message *, const char *, struct ldb_dn *)
+ldb_msg_add_steal_string: int (struct ldb_message *, const char *, char *)
+ldb_msg_add_steal_value: int (struct ldb_message *, const char *, struct ldb_val *)
+ldb_msg_add_string: int (struct ldb_message *, const char *, const char *)
+ldb_msg_add_value: int (struct ldb_message *, const char *, const struct ldb_val *, struct ldb_message_element **)
+ldb_msg_canonicalize: struct ldb_message *(struct ldb_context *, const struct ldb_message *)
+ldb_msg_check_string_attribute: int (const struct ldb_message *, const char *, const char *)
+ldb_msg_copy: struct ldb_message *(TALLOC_CTX *, const struct ldb_message *)
+ldb_msg_copy_attr: int (struct ldb_message *, const char *, const char *)
+ldb_msg_copy_shallow: struct ldb_message *(TALLOC_CTX *, const struct ldb_message *)
+ldb_msg_diff: struct ldb_message *(struct ldb_context *, struct ldb_message *, struct ldb_message *)
+ldb_msg_difference: int (struct ldb_context *, TALLOC_CTX *, struct ldb_message *, struct ldb_message *, struct ldb_message **)
+ldb_msg_element_compare: int (struct ldb_message_element *, struct ldb_message_element *)
+ldb_msg_element_compare_name: int (struct ldb_message_element *, struct ldb_message_element *)
+ldb_msg_element_equal_ordered: bool (const struct ldb_message_element *, const struct ldb_message_element *)
+ldb_msg_find_attr_as_bool: int (const struct ldb_message *, const char *, int)
+ldb_msg_find_attr_as_dn: struct ldb_dn *(struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, const char *)
+ldb_msg_find_attr_as_double: double (const struct ldb_message *, const char *, double)
+ldb_msg_find_attr_as_int: int (const struct ldb_message *, const char *, int)
+ldb_msg_find_attr_as_int64: int64_t (const struct ldb_message *, const char *, int64_t)
+ldb_msg_find_attr_as_string: const char *(const struct ldb_message *, const char *, const char *)
+ldb_msg_find_attr_as_uint: unsigned int (const struct ldb_message *, const char *, unsigned int)
+ldb_msg_find_attr_as_uint64: uint64_t (const struct ldb_message *, const char *, uint64_t)
+ldb_msg_find_common_values: int (struct ldb_context *, TALLOC_CTX *, struct ldb_message_element *, struct ldb_message_element *, uint32_t)
+ldb_msg_find_duplicate_val: int (struct ldb_context *, TALLOC_CTX *, const struct ldb_message_element *, struct ldb_val **, uint32_t)
+ldb_msg_find_element: struct ldb_message_element *(const struct ldb_message *, const char *)
+ldb_msg_find_ldb_val: const struct ldb_val *(const struct ldb_message *, const char *)
+ldb_msg_find_val: struct ldb_val *(const struct ldb_message_element *, struct ldb_val *)
+ldb_msg_new: struct ldb_message *(TALLOC_CTX *)
+ldb_msg_normalize: int (struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_message **)
+ldb_msg_remove_attr: void (struct ldb_message *, const char *)
+ldb_msg_remove_element: void (struct ldb_message *, struct ldb_message_element *)
+ldb_msg_rename_attr: int (struct ldb_message *, const char *, const char *)
+ldb_msg_sanity_check: int (struct ldb_context *, const struct ldb_message *)
+ldb_msg_sort_elements: void (struct ldb_message *)
+ldb_next_del_trans: int (struct ldb_module *)
+ldb_next_end_trans: int (struct ldb_module *)
+ldb_next_init: int (struct ldb_module *)
+ldb_next_prepare_commit: int (struct ldb_module *)
+ldb_next_read_lock: int (struct ldb_module *)
+ldb_next_read_unlock: int (struct ldb_module *)
+ldb_next_remote_request: int (struct ldb_module *, struct ldb_request *)
+ldb_next_request: int (struct ldb_module *, struct ldb_request *)
+ldb_next_start_trans: int (struct ldb_module *)
+ldb_op_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_options_find: const char *(struct ldb_context *, const char **, const char *)
+ldb_pack_data: int (struct ldb_context *, const struct ldb_message *, struct ldb_val *)
+ldb_parse_control_from_string: struct ldb_control *(struct ldb_context *, TALLOC_CTX *, const char *)
+ldb_parse_control_strings: struct ldb_control **(struct ldb_context *, TALLOC_CTX *, const char **)
+ldb_parse_tree: struct ldb_parse_tree *(TALLOC_CTX *, const char *)
+ldb_parse_tree_attr_replace: void (struct ldb_parse_tree *, const char *, const char *)
+ldb_parse_tree_copy_shallow: struct ldb_parse_tree *(TALLOC_CTX *, const struct ldb_parse_tree *)
+ldb_parse_tree_walk: int (struct ldb_parse_tree *, int (*)(struct ldb_parse_tree *, void *), void *)
+ldb_qsort: void (void * const, size_t, size_t, void *, ldb_qsort_cmp_fn_t)
+ldb_register_backend: int (const char *, ldb_connect_fn, bool)
+ldb_register_extended_match_rule: int (struct ldb_context *, const struct ldb_extended_match_rule *)
+ldb_register_hook: int (ldb_hook_fn)
+ldb_register_module: int (const struct ldb_module_ops *)
+ldb_rename: int (struct ldb_context *, struct ldb_dn *, struct ldb_dn *)
+ldb_reply_add_control: int (struct ldb_reply *, const char *, bool, void *)
+ldb_reply_get_control: struct ldb_control *(struct ldb_reply *, const char *)
+ldb_req_get_custom_flags: uint32_t (struct ldb_request *)
+ldb_req_is_untrusted: bool (struct ldb_request *)
+ldb_req_location: const char *(struct ldb_request *)
+ldb_req_mark_trusted: void (struct ldb_request *)
+ldb_req_mark_untrusted: void (struct ldb_request *)
+ldb_req_set_custom_flags: void (struct ldb_request *, uint32_t)
+ldb_req_set_location: void (struct ldb_request *, const char *)
+ldb_request: int (struct ldb_context *, struct ldb_request *)
+ldb_request_add_control: int (struct ldb_request *, const char *, bool, void *)
+ldb_request_done: int (struct ldb_request *, int)
+ldb_request_get_control: struct ldb_control *(struct ldb_request *, const char *)
+ldb_request_get_status: int (struct ldb_request *)
+ldb_request_replace_control: int (struct ldb_request *, const char *, bool, void *)
+ldb_request_set_state: void (struct ldb_request *, int)
+ldb_reset_err_string: void (struct ldb_context *)
+ldb_save_controls: int (struct ldb_control *, struct ldb_request *, struct ldb_control ***)
+ldb_schema_attribute_add: int (struct ldb_context *, const char *, unsigned int, const char *)
+ldb_schema_attribute_add_with_syntax: int (struct ldb_context *, const char *, unsigned int, const struct ldb_schema_syntax *)
+ldb_schema_attribute_by_name: const struct ldb_schema_attribute *(struct ldb_context *, const char *)
+ldb_schema_attribute_fill_with_syntax: int (struct ldb_context *, TALLOC_CTX *, const char *, unsigned int, const struct ldb_schema_syntax *, struct ldb_schema_attribute *)
+ldb_schema_attribute_remove: void (struct ldb_context *, const char *)
+ldb_schema_attribute_remove_flagged: void (struct ldb_context *, unsigned int)
+ldb_schema_attribute_set_override_handler: void (struct ldb_context *, ldb_attribute_handler_override_fn_t, void *)
+ldb_schema_set_override_indexlist: void (struct ldb_context *, bool)
+ldb_search: int (struct ldb_context *, TALLOC_CTX *, struct ldb_result **, struct ldb_dn *, enum ldb_scope, const char * const *, const char *, ...)
+ldb_search_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_sequence_number: int (struct ldb_context *, enum ldb_sequence_type, uint64_t *)
+ldb_set_create_perms: void (struct ldb_context *, unsigned int)
+ldb_set_debug: int (struct ldb_context *, void (*)(void *, enum ldb_debug_level, const char *, va_list), void *)
+ldb_set_debug_stderr: int (struct ldb_context *)
+ldb_set_default_dns: void (struct ldb_context *)
+ldb_set_errstring: void (struct ldb_context *, const char *)
+ldb_set_event_context: void (struct ldb_context *, struct tevent_context *)
+ldb_set_flags: void (struct ldb_context *, unsigned int)
+ldb_set_modules_dir: void (struct ldb_context *, const char *)
+ldb_set_opaque: int (struct ldb_context *, const char *, void *)
+ldb_set_require_private_event_context: void (struct ldb_context *)
+ldb_set_timeout: int (struct ldb_context *, struct ldb_request *, int)
+ldb_set_timeout_from_prev_req: int (struct ldb_context *, struct ldb_request *, struct ldb_request *)
+ldb_set_utf8_default: void (struct ldb_context *)
+ldb_set_utf8_fns: void (struct ldb_context *, void *, char *(*)(void *, void *, const char *, size_t))
+ldb_setup_wellknown_attributes: int (struct ldb_context *)
+ldb_should_b64_encode: int (struct ldb_context *, const struct ldb_val *)
+ldb_standard_syntax_by_name: const struct ldb_schema_syntax *(struct ldb_context *, const char *)
+ldb_strerror: const char *(int)
+ldb_string_to_time: time_t (const char *)
+ldb_string_utc_to_time: time_t (const char *)
+ldb_timestring: char *(TALLOC_CTX *, time_t)
+ldb_timestring_utc: char *(TALLOC_CTX *, time_t)
+ldb_transaction_cancel: int (struct ldb_context *)
+ldb_transaction_cancel_noerr: int (struct ldb_context *)
+ldb_transaction_commit: int (struct ldb_context *)
+ldb_transaction_prepare_commit: int (struct ldb_context *)
+ldb_transaction_start: int (struct ldb_context *)
+ldb_unpack_data: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *)
+ldb_unpack_data_only_attr_list: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *, const char * const *, unsigned int, unsigned int *)
+ldb_unpack_data_only_attr_list_flags: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *, const char * const *, unsigned int, unsigned int, unsigned int *)
+ldb_val_dup: struct ldb_val (TALLOC_CTX *, const struct ldb_val *)
+ldb_val_equal_exact: int (const struct ldb_val *, const struct ldb_val *)
+ldb_val_map_local: struct ldb_val (struct ldb_module *, void *, const struct ldb_map_attribute *, const struct ldb_val *)
+ldb_val_map_remote: struct ldb_val (struct ldb_module *, void *, const struct ldb_map_attribute *, const struct ldb_val *)
+ldb_val_string_cmp: int (const struct ldb_val *, const char *)
+ldb_val_to_time: int (const struct ldb_val *, time_t *)
+ldb_valid_attr_name: int (const char *)
+ldb_vdebug: void (struct ldb_context *, enum ldb_debug_level, const char *, va_list)
+ldb_wait: int (struct ldb_handle *, enum ldb_wait_type)
diff --git a/lib/ldb/ABI/pyldb-util-1.2.0.sigs b/lib/ldb/ABI/pyldb-util-1.2.0.sigs
new file mode 100644
index 0000000..74d6719
--- /dev/null
+++ b/lib/ldb/ABI/pyldb-util-1.2.0.sigs
@@ -0,0 +1,2 @@
+pyldb_Dn_FromDn: PyObject *(struct ldb_dn *)
+pyldb_Object_AsDn: bool (TALLOC_CTX *, PyObject *, struct ldb_context *, struct ldb_dn **)
diff --git a/lib/ldb/ABI/pyldb-util.py3-1.2.0.sigs b/lib/ldb/ABI/pyldb-util.py3-1.2.0.sigs
new file mode 100644
index 0000000..74d6719
--- /dev/null
+++ b/lib/ldb/ABI/pyldb-util.py3-1.2.0.sigs
@@ -0,0 +1,2 @@
+pyldb_Dn_FromDn: PyObject *(struct ldb_dn *)
+pyldb_Object_AsDn: bool (TALLOC_CTX *, PyObject *, struct ldb_context *, struct ldb_dn **)
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index 2e77f00..6f36965 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -1,7 +1,7 @@
#!/usr/bin/env python
APPNAME = 'ldb'
-VERSION = '1.1.31'
+VERSION = '1.2.0'
blddir = 'bin'
--
2.9.4
From 40f5b5d13e1b7b843d8774ab402f723db1d4fa89 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 16 Jun 2017 15:49:45 +1200
Subject: [PATCH 24/35] WRONG TEST dsdb: Add test showing that the DB is locked
during a search
Pair-programmed-with: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
python/samba/tests/dsdb.py | 52 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index 4a34bac..d7b8ecf 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -26,7 +26,8 @@ from samba.dcerpc import drsblobs
import ldb
import os
import samba
-
+import gc
+import time
class DsdbTests(TestCase):
@@ -149,3 +150,52 @@ class DsdbTests(TestCase):
version = self.samdb.get_attribute_replmetadata_version(dn, "description")
self.samdb.set_attribute_replmetadata_version(dn, "description", version + 2)
self.assertEqual(self.samdb.get_attribute_replmetadata_version(dn, "description"), version + 2)
+
+ def test_db_lock(self):
+ basedn = self.samdb.get_default_basedn()
+ (r1, w1) = os.pipe()
+
+ pid = os.fork()
+ if pid == 0:
+ # In the child, close the main DB, re-open just one DB
+ del(self.samdb)
+ gc.collect()
+ self.samdb = SamDB(session_info=self.session,
+ credentials=self.creds,
+ lp=self.lp)
+
+ self.samdb.transaction_start()
+
+ self.samdb.add({
+ "dn": "cn=test_db_lock_user,cn=users," + str(basedn),
+ "objectclass": "user",
+ })
+
+ # Obtain a write lock
+ self.samdb.transaction_prepare_commit()
+ os.write(w1, b"added")
+ time.sleep(2)
+
+ # Drop the write lock
+ self.samdb.transaction_cancel()
+ os._exit(0)
+
+ self.assertEqual(os.read(r1, 5), b"added")
+
+ start = time.time()
+
+ # We need to hold this iterator open to hold the all-record lock.
+ res = self.samdb.search_iterator()
+
+ # This should take at least 2 seconds because the transaction
+ # has a write lock on one backend db open
+
+ # Release the locks
+ for l in res:
+ pass
+
+ end = time.time()
+ self.assertGreater(end - start, 1.9)
+
+ (got_pid, status) = os.waitpid(pid, 0)
+ self.assertEqual(got_pid, pid)
--
2.9.4
From b2de57e857b7fd43f6b9d8a85d15446dd2c5d933 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 16 Jun 2017 15:49:45 +1200
Subject: [PATCH 25/35] WRONG TEST dsdb: Add test showing that the whole DB
(including partitions) is locked during a search
Pair-programmed-with: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
python/samba/tests/dsdb.py | 52 ++++++++++++++++++++++++++++++++++++++++
selftest/knownfail.d/ldb-locking | 1 +
2 files changed, 53 insertions(+)
create mode 100644 selftest/knownfail.d/ldb-locking
diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index d7b8ecf..9abc7fd 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -199,3 +199,55 @@ class DsdbTests(TestCase):
(got_pid, status) = os.waitpid(pid, 0)
self.assertEqual(got_pid, pid)
+
+
+ def test_full_db_lock(self):
+ basedn = self.samdb.get_default_basedn()
+ backend_filename = "%s.ldb" % basedn.get_casefold()
+ backend_subpath = os.path.join("sam.ldb.d",
+ backend_filename)
+ backend_path = self.lp.private_path(backend_subpath)
+ (r1, w1) = os.pipe()
+
+ pid = os.fork()
+ if pid == 0:
+ # In the child, close the main DB, re-open just one DB
+ del(self.samdb)
+ gc.collect()
+
+ backenddb = ldb.Ldb(backend_path)
+
+
+ backenddb.transaction_start()
+
+ backenddb.add({"dn":"@DSDB_LOCK_TEST"})
+ backenddb.delete("@DSDB_LOCK_TEST")
+
+ # Obtain a write lock
+ backenddb.transaction_prepare_commit()
+ os.write(w1, b"added")
+ time.sleep(2)
+
+ # Drop the write lock
+ backenddb.transaction_cancel()
+ os._exit(0)
+
+ self.assertEqual(os.read(r1, 5), b"added")
+
+ start = time.time()
+
+ # We need to hold this iterator open to hold the all-record lock.
+ res = self.samdb.search_iterator()
+
+ # This should take at least 2 seconds because the transaction
+ # has a write lock on one backend db open
+
+ end = time.time()
+ self.assertGreater(end - start, 1.9)
+
+ # Release the locks
+ for l in res:
+ pass
+
+ (got_pid, status) = os.waitpid(pid, 0)
+ self.assertEqual(got_pid, pid)
diff --git a/selftest/knownfail.d/ldb-locking b/selftest/knownfail.d/ldb-locking
new file mode 100644
index 0000000..5b569d2
--- /dev/null
+++ b/selftest/knownfail.d/ldb-locking
@@ -0,0 +1 @@
+samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock\(ad_dc_ntvfs:local\)
--
2.9.4
From 23edb2ff22be110eb678f6f40d75dcc26ae992c3 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 23 Jun 2017 12:13:19 +0200
Subject: [PATCH 26/35] TODO SIGN OFF dsdb: Add more locking more tests,
confirming blocking locks in both directions
These extended tests allow us to show that a search (read) blocks a
transaction commit (write), and that a transaction commit blocks a
search.
Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
TODO: Signed-off-by: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/tests/dsdb.py | 172 +++++++++++++++++++++++++++++++++++++--
selftest/knownfail.d/ldb-locking | 4 +-
2 files changed, 168 insertions(+), 8 deletions(-)
diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index 9abc7fd..b47f56d 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -151,7 +151,7 @@ class DsdbTests(TestCase):
self.samdb.set_attribute_replmetadata_version(dn, "description", version + 2)
self.assertEqual(self.samdb.get_attribute_replmetadata_version(dn, "description"), version + 2)
- def test_db_lock(self):
+ def test_db_lock1(self):
basedn = self.samdb.get_default_basedn()
(r1, w1) = os.pipe()
@@ -166,21 +166,23 @@ class DsdbTests(TestCase):
self.samdb.transaction_start()
+ dn = "cn=test_db_lock_user,cn=users," + str(basedn)
self.samdb.add({
- "dn": "cn=test_db_lock_user,cn=users," + str(basedn),
+ "dn": dn,
"objectclass": "user",
})
+ self.samdb.delete(dn)
# Obtain a write lock
self.samdb.transaction_prepare_commit()
- os.write(w1, b"added")
+ os.write(w1, b"prepared")
time.sleep(2)
# Drop the write lock
self.samdb.transaction_cancel()
os._exit(0)
- self.assertEqual(os.read(r1, 5), b"added")
+ self.assertEqual(os.read(r1, 8), b"prepared")
start = time.time()
@@ -201,7 +203,83 @@ class DsdbTests(TestCase):
self.assertEqual(got_pid, pid)
- def test_full_db_lock(self):
+ def test_db_lock2(self):
+ basedn = self.samdb.get_default_basedn()
+ (r1, w1) = os.pipe()
+ (r2, w2) = os.pipe()
+
+ pid = os.fork()
+ if pid == 0:
+ # In the child, close the main DB, re-open
+ del(self.samdb)
+ gc.collect()
+ self.samdb = SamDB(session_info=self.session,
+ credentials=self.creds,
+ lp=self.lp)
+
+ # We need to hold this iterator open to hold the all-record lock.
+ res = self.samdb.search_iterator()
+
+ os.write(w2, b"start")
+ if (os.read(r1, 7) != b"started"):
+ os._exit(1)
+
+ os.write(w2, b"add")
+ if (os.read(r1, 5) != b"added"):
+ os._exit(2)
+
+ # Wait 2 seconds to block prepare_commit() in the child.
+ os.write(w2, b"prepare")
+ time.sleep(2)
+
+ # Release the locks
+ for l in res:
+ pass
+
+ if (os.read(r1, 8) != b"prepared"):
+ os._exit(3)
+
+ os._exit(0)
+
+ # We can start the transaction during the search
+ # because both just grab the all-record read lock.
+ self.assertEqual(os.read(r2, 5), b"start")
+ self.samdb.transaction_start()
+ os.write(w1, b"started")
+
+ self.assertEqual(os.read(r2, 3), b"add")
+ dn = "cn=test_db_lock_user,cn=users," + str(basedn)
+ self.samdb.add({
+ "dn": dn,
+ "objectclass": "user",
+ })
+ self.samdb.delete(dn)
+ os.write(w1, b"added")
+
+ # Obtain a write lock, this will block until
+ # the parent releases the read lock.
+ self.assertEqual(os.read(r2, 7), b"prepare")
+ start = time.time()
+ self.samdb.transaction_prepare_commit()
+ end = time.time()
+ try:
+ self.assertGreater(end - start, 1.9)
+ except:
+ raise
+ finally:
+ os.write(w1, b"prepared")
+
+ # Drop the write lock
+ self.samdb.transaction_cancel()
+
+ (got_pid, status) = os.waitpid(pid, 0)
+ self.assertTrue(os.WIFEXITED(status))
+ self.assertEqual(os.WEXITSTATUS(status), 0)
+ self.assertEqual(got_pid, pid)
+
+
+
+ def test_full_db_lock1(self):
basedn = self.samdb.get_default_basedn()
backend_filename = "%s.ldb" % basedn.get_casefold()
backend_subpath = os.path.join("sam.ldb.d",
@@ -225,14 +303,14 @@ class DsdbTests(TestCase):
# Obtain a write lock
backenddb.transaction_prepare_commit()
- os.write(w1, b"added")
+ os.write(w1, b"prepared")
time.sleep(2)
# Drop the write lock
backenddb.transaction_cancel()
os._exit(0)
- self.assertEqual(os.read(r1, 5), b"added")
+ self.assertEqual(os.read(r1, 8), b"prepared")
start = time.time()
@@ -251,3 +329,83 @@ class DsdbTests(TestCase):
(got_pid, status) = os.waitpid(pid, 0)
self.assertEqual(got_pid, pid)
+
+ def test_full_db_lock2(self):
+ basedn = self.samdb.get_default_basedn()
+ backend_filename = "%s.ldb" % basedn.get_casefold()
+ backend_subpath = os.path.join("sam.ldb.d",
+ backend_filename)
+ backend_path = self.lp.private_path(backend_subpath)
+ (r1, w1) = os.pipe()
+ (r2, w2) = os.pipe()
+
+ pid = os.fork()
+ if pid == 0:
+
+ # In the child, close the main DB, re-open
+ del(self.samdb)
+ gc.collect()
+ self.samdb = SamDB(session_info=self.session,
+ credentials=self.creds,
+ lp=self.lp)
+
+ # We need to hold this iterator open to hold the all-record lock.
+ res = self.samdb.search_iterator()
+
+ os.write(w2, b"start")
+ if (os.read(r1, 7) != b"started"):
+ os._exit(1)
+ os.write(w2, b"add")
+ if (os.read(r1, 5) != b"added"):
+ os._exit(2)
+
+ # Wait 2 seconds to block prepare_commit() in the child.
+ os.write(w2, b"prepare")
+ time.sleep(2)
+
+ # Release the locks
+ for l in res:
+ pass
+
+ if (os.read(r1, 8) != b"prepared"):
+ os._exit(3)
+
+ os._exit(0)
+
+ # In the parent, close the main DB, re-open just one DB
+ del(self.samdb)
+ gc.collect()
+ backenddb = ldb.Ldb(backend_path)
+
+ # We can start the transaction during the search
+ # because both just grab the all-record read lock.
+ self.assertEqual(os.read(r2, 5), b"start")
+ backenddb.transaction_start()
+ os.write(w1, b"started")
+
+ self.assertEqual(os.read(r2, 3), b"add")
+ backenddb.add({"dn":"@DSDB_LOCK_TEST"})
+ backenddb.delete("@DSDB_LOCK_TEST")
+ os.write(w1, b"added")
+
+ # Obtain a write lock, this will block until
+ # the child releases the read lock.
+ self.assertEqual(os.read(r2, 7), b"prepare")
+ start = time.time()
+ backenddb.transaction_prepare_commit()
+ end = time.time()
+
+ try:
+ self.assertGreater(end - start, 1.9)
+ except:
+ raise
+ finally:
+ os.write(w1, b"prepared")
+
+ # Drop the write lock
+ backenddb.transaction_cancel()
+
+ (got_pid, status) = os.waitpid(pid, 0)
+ self.assertEqual(got_pid, pid)
+ self.assertTrue(os.WIFEXITED(status))
+ self.assertEqual(os.WEXITSTATUS(status), 0)
diff --git a/selftest/knownfail.d/ldb-locking b/selftest/knownfail.d/ldb-locking
index 5b569d2..81cc981 100644
--- a/selftest/knownfail.d/ldb-locking
+++ b/selftest/knownfail.d/ldb-locking
@@ -1 +1,3 @@
-samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock\(ad_dc_ntvfs:local\)
+samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock1\(ad_dc_ntvfs:local\)
+samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock2\(ad_dc_ntvfs:local\)
+samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_db_lock2\(ad_dc_ntvfs:local\)
\ No newline at end of file
--
2.9.4
From 30e198d9229fe9858960c79cc2d2a08f53bbd213 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 26 Jun 2017 13:16:01 +1200
Subject: [PATCH 27/35] dsdb: Add new test adding a record to the top level
sam.ldb file
This shows that locks are made on this file as well
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/tests/dsdb.py | 74 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index b47f56d..acba2fb 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -279,6 +279,77 @@ class DsdbTests(TestCase):
+ def test_db_lock3(self):
+ basedn = self.samdb.get_default_basedn()
+ (r1, w1) = os.pipe()
+ (r2, w2) = os.pipe()
+
+ pid = os.fork()
+ if pid == 0:
+ # In the child, close the main DB, re-open
+ del(self.samdb)
+ gc.collect()
+ self.samdb = SamDB(session_info=self.session,
+ credentials=self.creds,
+ lp=self.lp)
+
+ # We need to hold this iterator open to hold the all-record lock.
+ res = self.samdb.search_iterator()
+
+ os.write(w2, b"start")
+ if (os.read(r1, 7) != b"started"):
+ os._exit(1)
+
+ os.write(w2, b"add")
+ if (os.read(r1, 5) != b"added"):
+ os._exit(2)
+
+ # Wait 2 seconds to block prepare_commit() in the child.
+ os.write(w2, b"prepare")
+ time.sleep(2)
+
+ # Release the locks
+ for l in res:
+ pass
+
+ if (os.read(r1, 8) != b"prepared"):
+ os._exit(3)
+
+ os._exit(0)
+
+ # We can start the transaction during the search
+ # because both just grab the all-record read lock.
+ self.assertEqual(os.read(r2, 5), b"start")
+ self.samdb.transaction_start()
+ os.write(w1, b"started")
+
+ self.assertEqual(os.read(r2, 3), b"add")
+
+ # This will end up in the top level db
+ dn = "@DSDB_LOCK_TEST"
+ self.samdb.add({
+ "dn": dn})
+ self.samdb.delete(dn)
+ os.write(w1, b"added")
+
+ # Obtain a write lock, this will block until
+ # the child releases the read lock.
+ self.assertEqual(os.read(r2, 7), b"prepare")
+ start = time.time()
+ self.samdb.transaction_prepare_commit()
+ end = time.time()
+ self.assertGreater(end - start, 1.9)
+ os.write(w1, b"prepared")
+
+ # Drop the write lock
+ self.samdb.transaction_cancel()
+
+ (got_pid, status) = os.waitpid(pid, 0)
+ self.assertTrue(os.WIFEXITED(status))
+ self.assertEqual(os.WEXITSTATUS(status), 0)
+ self.assertEqual(got_pid, pid)
+
+
def test_full_db_lock1(self):
basedn = self.samdb.get_default_basedn()
backend_filename = "%s.ldb" % basedn.get_casefold()
@@ -329,6 +400,9 @@ class DsdbTests(TestCase):
(got_pid, status) = os.waitpid(pid, 0)
self.assertEqual(got_pid, pid)
+ self.assertTrue(os.WIFEXITED(status))
+ self.assertEqual(os.WEXITSTATUS(status), 0)
+
def test_full_db_lock2(self):
basedn = self.samdb.get_default_basedn()
--
2.9.4
From c72ee877ac6687111b3c65df29d727fd93d22c0f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 26 Jun 2017 13:20:54 +1200
Subject: [PATCH 28/35] dsdb: Add full checks on waitpid() results to
test_db_lock1
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/tests/dsdb.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index acba2fb..edce02a 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -201,6 +201,9 @@ class DsdbTests(TestCase):
(got_pid, status) = os.waitpid(pid, 0)
self.assertEqual(got_pid, pid)
+ self.assertTrue(os.WIFEXITED(status))
+ self.assertEqual(os.WEXITSTATUS(status), 0)
+ self.assertEqual(got_pid, pid)
def test_db_lock2(self):
--
2.9.4
From bd32e3ac8dfc5c0b1f118fa5644d6403518e5f96 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 26 Jun 2017 13:34:21 +1200
Subject: [PATCH 29/35] dsdb: Add tests showing that the CN=CONFIGURATION
partition is also locked
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/tests/dsdb.py | 40 +++++++++++++++++++++++++++++++++-------
selftest/knownfail.d/ldb-locking | 2 ++
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index edce02a..cab3c4b 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -353,12 +353,7 @@ class DsdbTests(TestCase):
self.assertEqual(got_pid, pid)
- def test_full_db_lock1(self):
- basedn = self.samdb.get_default_basedn()
- backend_filename = "%s.ldb" % basedn.get_casefold()
- backend_subpath = os.path.join("sam.ldb.d",
- backend_filename)
- backend_path = self.lp.private_path(backend_subpath)
+ def _test_full_db_lock1(self, backend_path):
(r1, w1) = os.pipe()
pid = os.fork()
@@ -407,12 +402,25 @@ class DsdbTests(TestCase):
self.assertEqual(os.WEXITSTATUS(status), 0)
- def test_full_db_lock2(self):
+ def test_full_db_lock1(self):
basedn = self.samdb.get_default_basedn()
backend_filename = "%s.ldb" % basedn.get_casefold()
backend_subpath = os.path.join("sam.ldb.d",
backend_filename)
backend_path = self.lp.private_path(backend_subpath)
+ self._test_full_db_lock1(backend_path)
+
+
+ def test_full_db_lock1_config(self):
+ basedn = self.samdb.get_config_basedn()
+ backend_filename = "%s.ldb" % basedn.get_casefold()
+ backend_subpath = os.path.join("sam.ldb.d",
+ backend_filename)
+ backend_path = self.lp.private_path(backend_subpath)
+ self._test_full_db_lock1(backend_path)
+
+
+ def _test_full_db_lock2(self, backend_path):
(r1, w1) = os.pipe()
(r2, w2) = os.pipe()
@@ -486,3 +494,21 @@ class DsdbTests(TestCase):
self.assertEqual(got_pid, pid)
self.assertTrue(os.WIFEXITED(status))
self.assertEqual(os.WEXITSTATUS(status), 0)
+
+
+ def test_full_db_lock2(self):
+ basedn = self.samdb.get_default_basedn()
+ backend_filename = "%s.ldb" % basedn.get_casefold()
+ backend_subpath = os.path.join("sam.ldb.d",
+ backend_filename)
+ backend_path = self.lp.private_path(backend_subpath)
+ self._test_full_db_lock2(backend_path)
+
+
+ def test_full_db_lock2_config(self):
+ basedn = self.samdb.get_config_basedn()
+ backend_filename = "%s.ldb" % basedn.get_casefold()
+ backend_subpath = os.path.join("sam.ldb.d",
+ backend_filename)
+ backend_path = self.lp.private_path(backend_subpath)
+ self._test_full_db_lock2(backend_path)
diff --git a/selftest/knownfail.d/ldb-locking b/selftest/knownfail.d/ldb-locking
index 81cc981..4245481 100644
--- a/selftest/knownfail.d/ldb-locking
+++ b/selftest/knownfail.d/ldb-locking
@@ -1,3 +1,5 @@
samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock1\(ad_dc_ntvfs:local\)
+samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock1_config\(ad_dc_ntvfs:local\)
samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock2\(ad_dc_ntvfs:local\)
+samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock2_config\(ad_dc_ntvfs:local\)
samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_db_lock2\(ad_dc_ntvfs:local\)
\ No newline at end of file
--
2.9.4
From 750cb04d04f4e8301bf9239ad9e5aedbe26f387c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 26 Jun 2017 14:13:41 +1200
Subject: [PATCH 30/35] dsdb: Teach the Samba partition module how to lock all
the DB backends
Note that locking the metadata partition (sam.ldb) is enough
to block another process in prepare_commit(), this change
is just performance optimizations in order to avoid multiple
lock/unlock (fcntl locks) sequences during the high level search.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
selftest/knownfail.d/ldb-locking | 5 -
source4/dsdb/samdb/ldb_modules/partition.c | 177 ++++++++++++++++++++++++++++-
2 files changed, 176 insertions(+), 6 deletions(-)
delete mode 100644 selftest/knownfail.d/ldb-locking
diff --git a/selftest/knownfail.d/ldb-locking b/selftest/knownfail.d/ldb-locking
deleted file mode 100644
index 4245481..0000000
--- a/selftest/knownfail.d/ldb-locking
+++ /dev/null
@@ -1,5 +0,0 @@
-samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock1\(ad_dc_ntvfs:local\)
-samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock1_config\(ad_dc_ntvfs:local\)
-samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock2\(ad_dc_ntvfs:local\)
-samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_full_db_lock2_config\(ad_dc_ntvfs:local\)
-samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_db_lock2\(ad_dc_ntvfs:local\)
\ No newline at end of file
diff --git a/source4/dsdb/samdb/ldb_modules/partition.c b/source4/dsdb/samdb/ldb_modules/partition.c
index 08a959c..568c71e 100644
--- a/source4/dsdb/samdb/ldb_modules/partition.c
+++ b/source4/dsdb/samdb/ldb_modules/partition.c
@@ -814,7 +814,7 @@ static int partition_start_trans(struct ldb_module *module)
ldb_debug(ldb_module_get_ctx(module), LDB_DEBUG_TRACE, "partition_start_trans() -> (metadata partition)");
}
- /* This order must match that in prepare_commit() */
+ /* This order must match that in prepare_commit() and read_lock() */
ret = ldb_next_start_trans(module);
if (ret != LDB_SUCCESS) {
return ret;
@@ -1144,6 +1144,179 @@ static int partition_sequence_number(struct ldb_module *module, struct ldb_reque
return ldb_module_done(req, NULL, ext, LDB_SUCCESS);
}
+/* lock all the backends */
+static int partition_read_lock(struct ldb_module *module)
+{
+ int i;
+ int ret;
+ int ret2;
+ struct ldb_context *ldb = ldb_module_get_ctx(module);
+ struct partition_private_data *data = \
+ talloc_get_type(ldb_module_get_private(module),
+ struct partition_private_data);
+
+ if (ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING) {
+ ldb_debug(ldb, LDB_DEBUG_TRACE,
+ "partition_read_lock() -> (metadata partition)");
+ }
+
+ /*
+ * It is important to only do this for LOCK because:
+ * - we don't want to unlock what we did not lock
+ *
+ * - we don't want to make a new lock on the sam.ldb
+ * (triggered inside this routine due to the seq num check)
+ * during an unlock phase as that will violate the lock
+ * ordering
+ */
+
+ /*
+ * This will lock the metadata partition (sam.ldb) and
+ * will also call event loops, so we do it before we
+ * get the whole db lock.
+ */
+ ret = partition_reload_if_required(module, data, NULL);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ /*
+ * This order must match that in prepare_commit(), start with
+ * the metadata partition (sam.ldb) lock
+ */
+ ret = ldb_next_read_lock(module);
+ if (ret != LDB_SUCCESS) {
+ ldb_debug_set(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to lock db: %s / %s for metadata partition",
+ ldb_errstring(ldb),
+ ldb_strerror(ret));
+
+ return ret;
+ }
+
+ /*
+ * As the metadata partition (sam.ldb) lock is enough
+ * to block another process in prepare_commit().
+ *
+ * That means the following (always uncontended) per partition locks
+ * are just performance optimizations in order to avoid multiple
+ * lock/unlock (fcntl locks) sequences during the high level search.
+ */
+ for (i=0; data && data->partitions && data->partitions[i]; i++) {
+ if ((module && ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING)) {
+ ldb_debug(ldb, LDB_DEBUG_TRACE,
+ "partition_read_lock() -> %s",
+ ldb_dn_get_linearized(
+ data->partitions[i]->ctrl->dn));
+ }
+ ret = ldb_next_read_lock(data->partitions[i]->module);
+ if (ret == LDB_SUCCESS) {
+ continue;
+ }
+
+ ldb_debug_set(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to lock db: %s / %s for %s",
+ ldb_errstring(ldb),
+ ldb_strerror(ret),
+ ldb_dn_get_linearized(
+ data->partitions[i]->ctrl->dn));
+
+ /* Back it out, if it fails on one */
+ for (i--; i >= 0; i--) {
+ ret2 = ldb_next_read_unlock(data->partitions[i]->module);
+ if (ret2 != LDB_SUCCESS) {
+ ldb_debug(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to unlock db: %s / %s",
+ ldb_errstring(ldb),
+ ldb_strerror(ret2));
+ }
+ }
+ ret2 = ldb_next_read_unlock(module);
+ if (ret2 != LDB_SUCCESS) {
+ ldb_debug(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to unlock db: %s / %s",
+ ldb_errstring(ldb),
+ ldb_strerror(ret2));
+ }
+ return ret;
+ }
+
+ return LDB_SUCCESS;
+}
+
+/* unlock all the backends */
+static int partition_read_unlock(struct ldb_module *module)
+{
+ int i;
+ int ret = LDB_SUCCESS;
+ int ret2;
+ struct ldb_context *ldb = ldb_module_get_ctx(module);
+ struct partition_private_data *data = \
+ talloc_get_type(ldb_module_get_private(module),
+ struct partition_private_data);
+
+ /*
+ * This order must be similar to partition_{end,del}_trans()
+ * the metadata partition (sam.ldb) unlock must be at the end.
+ */
+
+ for (i=0; data && data->partitions && data->partitions[i]; i++) {
+ if ((module && ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING)) {
+ ldb_debug(ldb, LDB_DEBUG_TRACE,
+ "partition_read_unlock() -> %s",
+ ldb_dn_get_linearized(
+ data->partitions[i]->ctrl->dn));
+ }
+ ret2 = ldb_next_read_unlock(data->partitions[i]->module);
+ if (ret2 != LDB_SUCCESS) {
+ ldb_debug_set(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to lock db: %s / %s for %s",
+ ldb_errstring(ldb),
+ ldb_strerror(ret),
+ ldb_dn_get_linearized(
+ data->partitions[i]->ctrl->dn));
+
+ /*
+ * Don't overwrite the original failure code
+ * if there was one
+ */
+ if (ret == LDB_SUCCESS) {
+ ret = ret2;
+ }
+ }
+ }
+
+ if (ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING) {
+ ldb_debug(ldb, LDB_DEBUG_TRACE,
+ "partition_read_unlock() -> (metadata partition)");
+ }
+
+ ret2 = ldb_next_read_unlock(module);
+ if (ret2 != LDB_SUCCESS) {
+ ldb_debug_set(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to unlock db: %s / %s for metadata partition",
+ ldb_errstring(ldb),
+ ldb_strerror(ret2));
+
+ /*
+ * Don't overwrite the original failure code
+ * if there was one
+ */
+ if (ret == LDB_SUCCESS) {
+ ret = ret2;
+ }
+ }
+
+ return ret;
+}
+
+
/* extended */
static int partition_extended(struct ldb_module *module, struct ldb_request *req)
{
@@ -1198,6 +1371,8 @@ static const struct ldb_module_ops ldb_partition_module_ops = {
.prepare_commit = partition_prepare_commit,
.end_transaction = partition_end_trans,
.del_transaction = partition_del_trans,
+ .read_lock = partition_read_lock,
+ .read_unlock = partition_read_unlock
};
int ldb_partition_module_init(const char *version)
--
2.9.4
From 6880a884744d2c1ddf11fbcdceb18f8534df1666 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 17 Oct 2016 13:55:42 +1300
Subject: [PATCH 31/35] ldap: Run the LDAP server with the default (typically
standard) process model
This allows one LDAP socket to proceed if another fails, and reduces the
impact of a crash becoming a DoS bug, as it only impacts one socket.
This may mean we have a lot of idle tasks, but this should not be a big
issue
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/ldap_server/ldap_server.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/source4/ldap_server/ldap_server.c b/source4/ldap_server/ldap_server.c
index 6a60532..d9f24e0 100644
--- a/source4/ldap_server/ldap_server.c
+++ b/source4/ldap_server/ldap_server.c
@@ -1121,9 +1121,12 @@ static void ldapsrv_task_init(struct task_server *task)
task_server_set_title(task, "task[ldapsrv]");
- /* run the ldap server as a single process */
- model_ops = process_model_startup("single");
- if (!model_ops) goto failed;
+ /*
+ * Here we used to run the ldap server as a single process,
+ * but we don't want transaction locks for one task in a write
+ * blocking all other reads, so we go multi-process.
+ */
+ model_ops = task->model_ops;
ldap_service = talloc_zero(task, struct ldapsrv_service);
if (ldap_service == NULL) goto failed;
--
2.9.4
From 24442cf8455cf502d65605987f16d68c57f56913 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 29 Jun 2017 12:50:03 +1200
Subject: [PATCH 32/35] WHATSNEW: Add an entry for the LDB whole DB locking
issue
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
WHATSNEW.txt | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index e8fbecb..7b0bac7 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -45,6 +45,37 @@ so it's still possible to connect to SMB1-only servers by default.
NEW FEATURES/CHANGES
====================
+Whole DB read locks: Improved LDAP and replication consistency
+--------------------------------------------------------------
+
+Prior to Samba 4.7 and ldb 1.2.0, the LDB database layer used by Samba
+erronously did not take whole-DB read locks to protect search
+and DRS replication operations.
+
+While each object returned remained subject to a record-level lock (so
+would remain consistent to itself), under a race condition with a
+rename or delete, it and any links (like the member attribute) to it
+would not be returned.
+
+The symptoms of this issue include:
+
+Replication failures with this error showing in the client side logs:
+ error during DRS repl ADD: No objectClass found in replPropertyMetaData for
+ Failed to commit objects:
+ WERR_GEN_FAILURE/NT_STATUS_INVALID_NETWORK_RESPONSE
+
+A crash of the server, in particular the rpc_server process with
+ INTERNAL ERROR: Signal 11
+
+LDAP read inconsistency
+ A DN subject to a search at the same time as it is being renamed
+ may not appear under either the old or new name, but will re-appear
+ for a subsequent search.
+
+See https://bugzilla.samba.org/show_bug.cgi?id=12858 for more details
+and updated advise on database recovery for affected installations.
+
+
Samba AD with MIT Kerberos
--------------------------
--
2.9.4
From d61e28b10433567200ac72e9314011bf7af34880 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 30 Jun 2017 16:02:46 +1200
Subject: [PATCH 33/35] WHATSNEW: Add entry for Multi-process LDAP Server
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
WHATSNEW.txt | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index 7b0bac7..dbca75e 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -136,6 +136,16 @@ authentication, SMB and RPC authorization is covered, however password
changes are not at this stage, and this support is not currently
backed by a testsuite.
+Multi-process LDAP Server
+-------------------------
+
+The LDAP server in the AD DC now honours the process model used for
+the rest of the samba process, rather than being forced into a single
+process. This aids in Samba's ability to scale to larger numbers of AD
+clients and the AD DC's overall resiliency, but will mean that there is a
+fork()ed child for every LDAP client, which may be more resource
+intensive in some situations.
+
Query record for open file or directory
---------------------------------------
--
2.9.4
From 350c3f46f5ffde66f5332355033e3706613c6c4b Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 28 Jun 2017 17:34:05 +1200
Subject: [PATCH 34/35] dsdb: Improve debugging on start transacton failure
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/repl/replicated_objects.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c
index 637f7fa..e5fec1f 100644
--- a/source4/dsdb/repl/replicated_objects.c
+++ b/source4/dsdb/repl/replicated_objects.c
@@ -842,7 +842,8 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
*/
ret = ldb_transaction_start(ldb);
if (ret != LDB_SUCCESS) {
- DEBUG(0,(__location__ " Failed to start transaction\n"));
+ DEBUG(0,(__location__ " Failed to start transaction: %s\n",
+ ldb_errstring(ldb)));
return WERR_FOOBAR;
}
--
2.9.4
From 5d44decd12e0cd6d8a2076819caeededcc2e6d01 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 3 May 2017 06:34:01 +0200
Subject: [PATCH 35/35] drsuapi: Improve debugging in DsAddEntry()
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/rpc_server/drsuapi/addentry.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/source4/rpc_server/drsuapi/addentry.c b/source4/rpc_server/drsuapi/addentry.c
index d071830..7cfb3fd 100644
--- a/source4/rpc_server/drsuapi/addentry.c
+++ b/source4/rpc_server/drsuapi/addentry.c
@@ -186,6 +186,8 @@ WERROR dcesrv_drsuapi_DsAddEntry(struct dcesrv_call_state *dce_call, TALLOC_CTX
case 2:
ret = ldb_transaction_start(b_state->sam_ctx);
if (ret != LDB_SUCCESS) {
+ DBG_ERR("DsAddEntry start transaction failed: %s\n",
+ ldb_errstring(b_state->sam_ctx));
return WERR_DS_DRA_INTERNAL_ERROR;
}
--
2.9.4
More information about the samba-technical
mailing list