tdb API issues
Howard Chu
hyc at highlandsun.com
Fri Apr 3 19:09:58 GMT 2009
Howard Chu wrote:
> Stefan (metze) Metzmacher wrote:
>>> I'm not sure yet that I've split things between the base_context and the
>>> caller context correctly; this is still all a work in progress but I
>>> wanted to get some early feedback.
>
>> I thought about a similar problem but without real threads,
>> we open the tdb more than once from within one process.
>>
>> I think in that case we should make sure that a transaction would only
>> be used from one caller.
>>
>> The idea was to let tdb_transaction_start() return EWOULDBLOCK
>> if a transaction was already started on a different tdb_context
>> (currently it's tdbwrap_context) (if a transaction is started in
>> a different process we would also return EWOULDBLOCK).
>
> That makes sense. My initial approach would allow this, but I didn't keep it:
> We simply add a have_transaction_lock flag in both the tdb_base_context and
> the tdb_context. If both are true, tdb_transaction_start simply nests as
> before. If tbc-> is true but tc-> is not, then return EWOULDBLOCK.
Try this patch instead of the 0002 I sent earlier. Again, this splits the
tdb_context structure; it also checks for tdb_transaction_start() conflicts
and tdb_traverse() conflicts. I think it accomplishes most of what you're
looking for.
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
-------------- next part --------------
diff --git a/lib/tdb/common/lock.c b/lib/tdb/common/lock.c
index 0ed63b3..640b32d 100644
--- a/lib/tdb/common/lock.c
+++ b/lib/tdb/common/lock.c
@@ -304,6 +304,12 @@ int tdb_transaction_lock(struct tdb_context *tdb, int ltype)
if (tdb->tc_have_transaction_lock || tdb->tc_global_lock.count) {
return 0;
}
+ if (tdb->tc_base->tbc_have_transaction_lock) {
+ TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_lock: transaction lock held by someone else\n"));
+ tdb->tc_ecode = TDB_ERR_LOCK;
+ errno = EWOULDBLOCK;
+ return -1;
+ }
if (tdb->tc_methods->tdb_brlock(tdb, TRANSACTION_LOCK, ltype,
F_SETLKW, 0, 1) == -1) {
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_lock: failed to get transaction lock\n"));
@@ -311,6 +317,7 @@ int tdb_transaction_lock(struct tdb_context *tdb, int ltype)
return -1;
}
tdb->tc_have_transaction_lock = 1;
+ tdb->tc_base->tbc_have_transaction_lock = 1;
return 0;
}
@@ -326,6 +333,7 @@ int tdb_transaction_unlock(struct tdb_context *tdb)
ret = tdb->tc_methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0, 1);
if (ret == 0) {
tdb->tc_have_transaction_lock = 0;
+ tdb->tc_base->tbc_have_transaction_lock = 0;
}
return ret;
}
diff --git a/lib/tdb/common/open.c b/lib/tdb/common/open.c
index 61c53ec..4884adb 100644
--- a/lib/tdb/common/open.c
+++ b/lib/tdb/common/open.c
@@ -159,7 +159,14 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
errno = ENOMEM;
goto fail;
}
+ if (!(tdb->tc_base = (struct tdb_base_context *)calloc(1,
+ sizeof *tdb->tc_base))) {
+ /* Can't log this */
+ errno = ENOMEM;
+ goto fail;
+ }
tdb_io_init(tdb);
+ tdb->tc_base->tbc_refcount = 1;
tdb->tc_fd = -1;
tdb->tc_name = NULL;
tdb->tc_map_ptr = NULL;
@@ -339,6 +346,7 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
if (tdb->tc_fd != -1)
if (close(tdb->tc_fd) != 0)
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_open_ex: failed to close tdb->tc_fd on error!\n"));
+ SAFE_FREE(tdb->tc_base);
SAFE_FREE(tdb);
errno = save_errno;
return NULL;
@@ -346,6 +354,26 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
}
/*
+ * Clone an open database, for multi-threaded access
+ */
+struct tdb_context *tdb_clone(struct tdb_context *tdb)
+{
+ struct tdb_context *tc2;
+
+ if (!(tc2 = (struct tdb_context *)calloc(1, sizeof *tc2))) {
+ /* Can't log this */
+ errno = ENOMEM;
+ return NULL;
+ }
+ tc2->tc_base = tdb->tc_base;
+ tdb->tc_base->tbc_refcount++;
+ tc2->tc_flags = tdb->tc_flags | TDB_CLONE;
+ tdb_io_init(tc2);
+ /* clones aren't in the tdbs list */
+ return tc2;
+}
+
+/*
* Set the maximum number of dead records per hash chain
*/
@@ -364,26 +392,33 @@ int tdb_close(struct tdb_context *tdb)
struct tdb_context **i;
int ret = 0;
- if (tdb->tc_transaction) {
+ if (tdb->tc_transaction && tdb->tc_in_transaction) {
tdb_transaction_cancel(tdb);
}
- if (tdb->tc_map_ptr) {
- if (tdb->tc_flags & TDB_INTERNAL)
- SAFE_FREE(tdb->tc_map_ptr);
- else
- tdb_munmap(tdb);
- }
- SAFE_FREE(tdb->tc_name);
- if (tdb->tc_fd != -1)
- ret = close(tdb->tc_fd);
- SAFE_FREE(tdb->tc_lockrecs);
-
- /* Remove from contexts list */
- for (i = &tdbs; *i; i = &(*i)->tc_next) {
- if (*i == tdb) {
- *i = tdb->tc_next;
- break;
+ if (--tdb->tc_base->tbc_refcount < 1) {
+ if (tdb->tc_map_ptr) {
+ if (tdb->tc_flags & TDB_INTERNAL)
+ SAFE_FREE(tdb->tc_map_ptr);
+ else
+ tdb_munmap(tdb);
+ }
+ SAFE_FREE(tdb->tc_name);
+ if (tdb->tc_fd != -1)
+ ret = close(tdb->tc_fd);
+ SAFE_FREE(tdb->tc_lockrecs);
+ memset(tdb->tc_base, 0, sizeof(*tdb->tc_base));
+ SAFE_FREE(tdb->tc_base);
+ }
+
+ /* clones aren't in the list */
+ if (!(tdb->tc_flags & TDB_CLONE)) {
+ /* Remove from contexts list */
+ for (i = &tdbs; *i; i = &(*i)->tc_next) {
+ if (*i == tdb) {
+ *i = tdb->tc_next;
+ break;
+ }
}
}
diff --git a/lib/tdb/common/tdb_private.h b/lib/tdb/common/tdb_private.h
index 311e9dd..fdc8d28 100644
--- a/lib/tdb/common/tdb_private.h
+++ b/lib/tdb/common/tdb_private.h
@@ -141,34 +141,63 @@ struct tdb_methods {
int (*tdb_brlock)(struct tdb_context *, tdb_off_t , int, int, int, size_t);
};
+struct tdb_base_context {
+ char *tbc_name; /* the name of the database */
+ void *tbc_map_ptr; /* where it is currently mapped */
+ int tbc_fd; /* open file descriptor for the database */
+ tdb_len_t tbc_map_size; /* how much space has been mapped */
+ int tbc_read_only; /* opened read-only */
+ int tbc_traverse_read; /* read-only traversal */
+ int tbc_traverse_write; /* read-write traversal */
+ struct tdb_lock_type tbc_global_lock;
+ int tbc_num_lockrecs;
+ struct tdb_lock_type *tbc_lockrecs; /* only real locks, all with count>0 */
+ struct tdb_header tbc_header; /* a cached copy of the header */
+ dev_t tbc_device; /* uniquely identifies this tdb */
+ ino_t tbc_inode; /* uniquely identifies this tdb */
+ struct tdb_logging_context tbc_log;
+ unsigned int (*tbc_hash_fn)(TDB_DATA *key);
+ int tbc_open_flags; /* flags used in the open - needed by reopen */
+ unsigned int tbc_num_locks; /* number of chain locks held */
+ struct tdb_transaction *tbc_transaction;
+ int tbc_page_size;
+ int tbc_max_dead_records;
+ bool tbc_have_transaction_lock;
+ int tbc_refcount;
+ volatile sig_atomic_t *tbc_interrupt_sig_ptr;
+};
+
+#define tc_name tc_base->tbc_name
+#define tc_map_ptr tc_base->tbc_map_ptr
+#define tc_fd tc_base->tbc_fd
+#define tc_map_size tc_base->tbc_map_size
+#define tc_read_only tc_base->tbc_read_only
+#define tc_traverse_read tc_base->tbc_traverse_read
+#define tc_traverse_write tc_base->tbc_traverse_write
+#define tc_global_lock tc_base->tbc_global_lock
+#define tc_num_lockrecs tc_base->tbc_num_lockrecs
+#define tc_lockrecs tc_base->tbc_lockrecs
+#define tc_header tc_base->tbc_header
+#define tc_device tc_base->tbc_device
+#define tc_inode tc_base->tbc_inode
+#define tc_log tc_base->tbc_log
+#define tc_hash_fn tc_base->tbc_hash_fn
+#define tc_open_flags tc_base->tbc_open_flags
+#define tc_num_locks tc_base->tbc_num_locks
+#define tc_page_size tc_base->tbc_page_size
+#define tc_max_dead_records tc_base->tbc_max_dead_records
+#define tc_interrupt_sig_ptr tc_base->tbc_interrupt_sig_ptr
+#define tc_transaction tc_base->tbc_transaction
+
struct tdb_context {
- char *tc_name; /* the name of the database */
- void *tc_map_ptr; /* where it is currently mapped */
- int tc_fd; /* open file descriptor for the database */
- tdb_len_t tc_map_size; /* how much space has been mapped */
- int tc_read_only; /* opened read-only */
- int tc_traverse_read; /* read-only traversal */
- int tc_traverse_write; /* read-write traversal */
- struct tdb_lock_type tc_global_lock;
- int tc_num_lockrecs;
- struct tdb_lock_type *tc_lockrecs; /* only real locks, all with count>0 */
- enum TDB_ERROR tc_ecode; /* error code for last tdb error */
- struct tdb_header tc_header; /* a cached copy of the header */
+ struct tdb_base_context *tc_base;
uint32_t tc_flags; /* the flags passed to tdb_open */
+ enum TDB_ERROR tc_ecode; /* error code for last tdb error */
struct tdb_traverse_lock tc_travlocks; /* current traversal locks */
struct tdb_context *tc_next; /* all tdbs to avoid multiple opens */
- dev_t tc_device; /* uniquely identifies this tdb */
- ino_t tc_inode; /* uniquely identifies this tdb */
- struct tdb_logging_context tc_log;
- unsigned int (*tc_hash_fn)(TDB_DATA *key);
- int tc_open_flags; /* flags used in the open - needed by reopen */
- unsigned int tc_num_locks; /* number of chain locks held */
const struct tdb_methods *tc_methods;
- struct tdb_transaction *tc_transaction;
- int tc_page_size;
- int tc_max_dead_records;
bool tc_have_transaction_lock;
- volatile sig_atomic_t *tc_interrupt_sig_ptr;
+ bool tc_in_transaction;
};
/*
diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index 385b3a2..c7839d6 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -429,6 +429,12 @@ int tdb_transaction_start(struct tdb_context *tdb)
/* cope with nested tdb_transaction_start() calls */
if (tdb->tc_transaction != NULL) {
+ if (!tdb->tc_in_transaction) {
+ TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_start: someone else already owns the transaction\n"));
+ tdb->tc_ecode = TDB_ERR_LOCK;
+ errno = EWOULDBLOCK;
+ return -1;
+ }
tdb->tc_transaction->nesting++;
TDB_LOG((tdb, TDB_DEBUG_TRACE, "tdb_transaction_start: nesting %d\n",
tdb->tc_transaction->nesting));
@@ -444,7 +450,7 @@ int tdb_transaction_start(struct tdb_context *tdb)
return -1;
}
- if (tdb->tc_travlocks.next != NULL) {
+ if (tdb->tc_traverse_read || tdb->tc_traverse_write) {
/* you cannot use transactions inside a traverse (although you can use
traverse inside a transaction) as otherwise you can end up with
deadlock */
@@ -504,6 +510,7 @@ int tdb_transaction_start(struct tdb_context *tdb)
transaction specific methods */
tdb->tc_transaction->io_methods = tdb->tc_methods;
tdb->tc_methods = &transaction_methods;
+ tdb->tc_in_transaction = true;
return 0;
@@ -607,6 +614,7 @@ int tdb_transaction_cancel(struct tdb_context *tdb)
tdb_transaction_unlock(tdb);
SAFE_FREE(tdb->tc_transaction->hash_heads);
SAFE_FREE(tdb->tc_transaction);
+ tdb->tc_in_transaction = false;
return ret;
}
diff --git a/lib/tdb/include/tdb.h b/lib/tdb/include/tdb.h
index c00b6cb..a2736b0 100644
--- a/lib/tdb/include/tdb.h
+++ b/lib/tdb/include/tdb.h
@@ -48,6 +48,7 @@ extern "C" {
#define TDB_NOSYNC 64 /* don't use synchronous transactions */
#define TDB_SEQNUM 128 /* maintain a sequence number */
#define TDB_VOLATILE 256 /* Activate the per-hashchain freelist, default 5 */
+#define TDB_CLONE 512 /* clone of existing context (internal use) */
/* error codes */
enum TDB_ERROR {TDB_SUCCESS=0, TDB_ERR_CORRUPT, TDB_ERR_IO, TDB_ERR_LOCK,
@@ -93,6 +94,7 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
int open_flags, mode_t mode,
const struct tdb_logging_context *log_ctx,
tdb_hash_func hash_fn);
+struct tdb_context *tdb_clone(struct tdb_context *tdb);
void tdb_set_max_dead(struct tdb_context *tdb, int max_dead);
int tdb_reopen(struct tdb_context *tdb);
More information about the samba-technical
mailing list