From 5027f9cd02bf322498313a6b7aabd03fd510d725 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 18 Jun 2012 22:30:27 +0930 Subject: 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 --- lib/ntdb/open.c | 53 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 23 deletions(-) (limited to 'lib/ntdb') 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)); -- cgit