summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRusty Russell <rusty@rustcorp.com.au>2012-06-18 22:30:27 +0930
committerRusty Russell <rusty@rustcorp.com.au>2012-06-19 05:38:06 +0200
commit5027f9cd02bf322498313a6b7aabd03fd510d725 (patch)
tree94d67ad070ec4039c4e90c029a771ecc4f9929b2
parent1765c0f9baf16ff1c2f5f109fb31e411831f0945 (diff)
downloadsamba-5027f9cd02bf322498313a6b7aabd03fd510d725.tar.gz
samba-5027f9cd02bf322498313a6b7aabd03fd510d725.tar.bz2
samba-5027f9cd02bf322498313a6b7aabd03fd510d725.zip
ntdb: reduce race between creating file and getting open lock.
In tdb, we grab the open lock immediately after we open the file. In ntdb, we usually did some work first. tdbtorture managed to get in before the creator grabbed the lock: testing with 3 processes, 5000 loops, seed=1338246020 ntdb:torture.ntdb:IO Error:ntdb_open: torture.ntdb is not a ntdb file 29023:torture.ntdb:db open failed At cost of a little duplicated code, we can reduce the race. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
-rw-r--r--lib/ntdb/open.c53
1 files changed, 30 insertions, 23 deletions
diff --git a/lib/ntdb/open.c b/lib/ntdb/open.c
index 28aff0a2ea..8dce4ce6f2 100644
--- a/lib/ntdb/open.c
+++ b/lib/ntdb/open.c
@@ -508,48 +508,55 @@ _PUBLIC_ struct ntdb_context *ntdb_open(const char *name, int ntdb_flags,
ntdb->file = find_file(st.st_dev, st.st_ino);
if (!ntdb->file) {
- int fd;
+ ecode = ntdb_new_file(ntdb);
+ if (ecode != NTDB_SUCCESS) {
+ goto fail;
+ }
- if ((fd = open(name, open_flags, mode)) == -1) {
+ /* Set this now, as ntdb_nest_lock examines it. */
+ ntdb->file->map_size = 0;
+
+ if ((ntdb->file->fd = open(name, open_flags, mode)) == -1) {
/* errno set by open(2) */
saved_errno = errno;
ntdb_logerr(ntdb, NTDB_ERR_IO, NTDB_LOG_ERROR,
"ntdb_open: could not open file %s: %s",
name, strerror(errno));
+
+ goto fail_errno;
+ }
+
+ /* ensure there is only one process initialising at once:
+ * do it immediately to reduce the create/openlock race. */
+ ecode = ntdb_lock_open(ntdb, openlock,
+ NTDB_LOCK_WAIT|NTDB_LOCK_NOCHECK);
+ if (ecode != NTDB_SUCCESS) {
+ saved_errno = errno;
goto fail_errno;
}
/* on exec, don't inherit the fd */
- v = fcntl(fd, F_GETFD, 0);
- fcntl(fd, F_SETFD, v | FD_CLOEXEC);
+ v = fcntl(ntdb->file->fd, F_GETFD, 0);
+ fcntl(ntdb->file->fd, F_SETFD, v | FD_CLOEXEC);
- if (fstat(fd, &st) == -1) {
+ if (fstat(ntdb->file->fd, &st) == -1) {
saved_errno = errno;
ntdb_logerr(ntdb, NTDB_ERR_IO, NTDB_LOG_ERROR,
"ntdb_open: could not stat open %s: %s",
name, strerror(errno));
- close(fd);
goto fail_errno;
}
- ecode = ntdb_new_file(ntdb);
- if (ecode != NTDB_SUCCESS) {
- close(fd);
- goto fail;
- }
-
- ntdb->file->fd = fd;
ntdb->file->device = st.st_dev;
ntdb->file->inode = st.st_ino;
- ntdb->file->map_ptr = NULL;
- ntdb->file->map_size = 0;
- }
-
- /* ensure there is only one process initialising at once */
- ecode = ntdb_lock_open(ntdb, openlock, NTDB_LOCK_WAIT|NTDB_LOCK_NOCHECK);
- if (ecode != NTDB_SUCCESS) {
- saved_errno = errno;
- goto fail_errno;
+ } else {
+ /* ensure there is only one process initialising at once */
+ ecode = ntdb_lock_open(ntdb, openlock,
+ NTDB_LOCK_WAIT|NTDB_LOCK_NOCHECK);
+ if (ecode != NTDB_SUCCESS) {
+ saved_errno = errno;
+ goto fail_errno;
+ }
}
/* call their open hook if they gave us one. */
@@ -696,7 +703,7 @@ fail_errno:
} else
ntdb_munmap(ntdb->file);
}
- if (close(ntdb->file->fd) != 0)
+ if (ntdb->file->fd != -1 && close(ntdb->file->fd) != 0)
ntdb_logerr(ntdb, NTDB_ERR_IO, NTDB_LOG_ERROR,
"ntdb_open: failed to close ntdb fd"
" on error: %s", strerror(errno));