From 013731c502a6cedbef0399ceb21dee1e34450222 Mon Sep 17 00:00:00 2001 From: Juan RP Date: Fri, 5 Sep 2014 12:26:42 +0200 Subject: [PATCH] Acquire/release a POSIX file lock on repository archives. - xbps_repo_open() accepts a third argument (bool) to acquire a POSIX file lock on the repository archive. - xbps_repo_close() accepts a second argument (bool) to release a POSIX file lock on the repository archive. This avoids the issue of multiple xbps-rindex(8) processes being blocked even for different repositories on the same architecture, resulting in unnecessary contention. --- NEWS | 6 +++ README.md | 1 - bin/xbps-rindex/Makefile | 2 +- bin/xbps-rindex/defs.h | 10 ---- bin/xbps-rindex/index-add.c | 22 ++++---- bin/xbps-rindex/index-clean.c | 18 +++---- bin/xbps-rindex/remove-obsoletes.c | 4 +- bin/xbps-rindex/sem.c | 81 ------------------------------ bin/xbps-rindex/sign.c | 10 +--- configure | 34 ------------- include/xbps.h.in | 12 +++-- lib/repo.c | 42 +++++++++++++--- lib/rpool.c | 2 +- 13 files changed, 70 insertions(+), 174 deletions(-) delete mode 100644 bin/xbps-rindex/sem.c diff --git a/NEWS b/NEWS index 8a4e3d82..8a8815db 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,11 @@ xbps-0.38 (???): + * libxbps: xbps_repo_{open,close}() gained an additional argument to acquire/release + a POSIX file lock (see lockf(3)) to serialize write access to the repository + archive. This obsoletes the POSIX named semaphores that were used in xbps-rindex(8), + which only allowed to add/clean a repository per architecture, even if it + wasn't the same local repository! + * libxbps: the fetch code now is able to connect to HTTPS without the need of /etc/services being available; We don't expect those ports to change anytime! diff --git a/README.md b/README.md index 8bc91ecb..31de52b4 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,6 @@ See `git tag -l` to list all available stable releases. To build this you'll need: - A C99 compiler (clang and gcc tested) - - A system that supports POSIX named semaphores - [GNU make](http://www.gnu.org/software/make/) - [pkg-config](http://www.freedesktop.org/wiki/Software/pkg-config/) - [zlib](http://www.zlib.net) diff --git a/bin/xbps-rindex/Makefile b/bin/xbps-rindex/Makefile index 0b404f11..2f17c90a 100644 --- a/bin/xbps-rindex/Makefile +++ b/bin/xbps-rindex/Makefile @@ -2,6 +2,6 @@ TOPDIR = ../.. -include $(TOPDIR)/config.mk BIN = xbps-rindex -OBJS = main.o sem.o index-add.o index-clean.o remove-obsoletes.o repoflush.o sign.o +OBJS = main.o index-add.o index-clean.o remove-obsoletes.o repoflush.o sign.o include $(TOPDIR)/mk/prog.mk diff --git a/bin/xbps-rindex/defs.h b/bin/xbps-rindex/defs.h index b2ce6668..52f99f1a 100644 --- a/bin/xbps-rindex/defs.h +++ b/bin/xbps-rindex/defs.h @@ -66,7 +66,6 @@ #endif #define _XBPS_RINDEX "xbps-rindex" -#define _XBPS_RINDEX_SEMNAME "/xbps-rindex-write" /* From index-add.c */ int index_add(struct xbps_handle *, int, char **, bool); @@ -85,13 +84,4 @@ int sign_repo(struct xbps_handle *, const char *, const char *, bool repodata_flush(struct xbps_handle *, const char *, xbps_dictionary_t, xbps_dictionary_t, xbps_dictionary_t); -struct idxlock { - sem_t *sem; - char *semname; -}; - -/* From sem.c */ -struct idxlock *index_lock(struct xbps_handle *); -void index_unlock(struct idxlock *); - #endif /* !_XBPS_RINDEX_DEFS_H_ */ diff --git a/bin/xbps-rindex/index-add.c b/bin/xbps-rindex/index-add.c index 169e4075..b04a2fc5 100644 --- a/bin/xbps-rindex/index-add.c +++ b/bin/xbps-rindex/index-add.c @@ -43,8 +43,7 @@ index_add(struct xbps_handle *xhp, int argc, char **argv, bool force) xbps_array_t array, pkg_files, pkg_links, pkg_cffiles; xbps_dictionary_t idx, idxmeta, idxfiles, binpkgd, pkg_filesd, curpkgd; xbps_object_t obj, fileobj; - struct idxlock *il; - struct xbps_repo *repo; + struct xbps_repo *repo = NULL; struct stat st; const char *arch; char *sha256, *pkgver, *opkgver, *oarch, *pkgname; @@ -52,23 +51,19 @@ index_add(struct xbps_handle *xhp, int argc, char **argv, bool force) int rv = 0, ret = 0; bool flush = false, found = false; - if ((il = index_lock(xhp)) == NULL) - return EINVAL; /* * Read the repository data or create index dictionaries otherwise. */ - if ((tmprepodir = strdup(argv[0])) == NULL) { - rv = ENOMEM; - goto out; - } + if ((tmprepodir = strdup(argv[0])) == NULL) + return ENOMEM; + repodir = dirname(tmprepodir); - repo = xbps_repo_open(xhp, repodir); + repo = xbps_repo_open(xhp, repodir, true); if (repo && repo->idx) { xbps_repo_open_idxfiles(repo); idx = xbps_dictionary_copy(repo->idx); idxmeta = xbps_dictionary_copy(repo->idxmeta); idxfiles = xbps_dictionary_copy(repo->idxfiles); - xbps_repo_close(repo); } else { idx = xbps_dictionary_create(); idxmeta = NULL; @@ -264,9 +259,10 @@ index_add(struct xbps_handle *xhp, int argc, char **argv, bool force) printf("index-files: %u packages registered.\n", xbps_dictionary_count(idxfiles)); out: - index_unlock(il); - if (tmprepodir) { + if (repo) + xbps_repo_close(repo, true); + if (tmprepodir) free(tmprepodir); - } + return rv; } diff --git a/bin/xbps-rindex/index-clean.c b/bin/xbps-rindex/index-clean.c index dd5b892c..b1273086 100644 --- a/bin/xbps-rindex/index-clean.c +++ b/bin/xbps-rindex/index-clean.c @@ -122,29 +122,24 @@ index_clean(struct xbps_handle *xhp, const char *repodir) xbps_dictionary_t idx = NULL, idxmeta = NULL, idxfiles = NULL; struct xbps_repo *repo; struct cbdata cbd; - struct idxlock *il; char *keyname, *pkgname; int rv = 0; bool flush = false; - if ((il = index_lock(xhp)) == NULL) - return EINVAL; - - repo = xbps_repo_open(xhp, repodir); + repo = xbps_repo_open(xhp, repodir, true); if (repo == NULL) { - if (errno == ENOENT) - goto out; + rv = errno; + if (rv == ENOENT) + return 0; fprintf(stderr, "%s: cannot read repository data: %s\n", _XBPS_RINDEX, strerror(errno)); - rv = errno; - goto out; + return rv; } xbps_repo_open_idxfiles(repo); idx = xbps_dictionary_copy(repo->idx); idxmeta = xbps_dictionary_copy(repo->idxmeta); idxfiles = xbps_dictionary_copy(repo->idxfiles); - xbps_repo_close(repo); if (idx == NULL || idxfiles == NULL) { fprintf(stderr, "%s: incomplete repository data file!\n", _XBPS_RINDEX); rv = EINVAL; @@ -207,8 +202,7 @@ index_clean(struct xbps_handle *xhp, const char *repodir) xbps_dictionary_count(idxfiles)); out: - index_unlock(il); - + xbps_repo_close(repo, true); if (idx) xbps_object_release(idx); if (idxmeta) diff --git a/bin/xbps-rindex/remove-obsoletes.c b/bin/xbps-rindex/remove-obsoletes.c index 0c1f222d..c5d1a6bb 100644 --- a/bin/xbps-rindex/remove-obsoletes.c +++ b/bin/xbps-rindex/remove-obsoletes.c @@ -117,7 +117,7 @@ remove_obsoletes(struct xbps_handle *xhp, const char *repodir) char *ext; int rv = 0; - repo = xbps_repo_open(xhp, repodir); + repo = xbps_repo_open(xhp, repodir, false); if (repo == NULL) { if (errno != ENOENT) { fprintf(stderr, "xbps-rindex: cannot read repository data: %s\n", @@ -151,7 +151,7 @@ remove_obsoletes(struct xbps_handle *xhp, const char *repodir) (void)closedir(dirp); rv = xbps_array_foreach_cb_multi(xhp, array, NULL, cleaner_cb, repo); - xbps_repo_close(repo); + xbps_repo_close(repo, false); xbps_object_release(array); return rv; diff --git a/bin/xbps-rindex/sem.c b/bin/xbps-rindex/sem.c deleted file mode 100644 index 736ad0ce..00000000 --- a/bin/xbps-rindex/sem.c +++ /dev/null @@ -1,81 +0,0 @@ -/*- - * Copyright (c) 2014 Juan Romero Pardines. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR - * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES - * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. - * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT - * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF - * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#include -#include -#include -#include -#include -#include - -#include "defs.h" - -struct idxlock * -index_lock(struct xbps_handle *xhp) -{ - struct idxlock *il; - mode_t myumask; - - if ((il = malloc(sizeof(struct idxlock))) == NULL) - return NULL; - - /* - * Generate semaphore name for target architecture. - */ - il->semname = xbps_xasprintf("/xbps-rindex-%s", - xhp->target_arch ? xhp->target_arch : xhp->native_arch); - /* - * Create/open the POSIX named semaphore. - */ - myumask = umask(0); - il->sem = sem_open(il->semname, O_CREAT, 0660, 1); - umask(myumask); - - if (il->sem == SEM_FAILED) { - fprintf(stderr, "%s: failed to create/open named " - "semaphore: %s\n", _XBPS_RINDEX, strerror(errno)); - free(il); - return NULL; - } - if (sem_wait(il->sem) == -1) { - fprintf(stderr, "%s: failed to lock named semaphore: %s\n", - _XBPS_RINDEX, strerror(errno)); - free(il); - return NULL; - } - - return il; -} - -void -index_unlock(struct idxlock *il) -{ - /* Unlock semaphore, close and destroy it (if possible) */ - sem_post(il->sem); - sem_close(il->sem); - sem_unlink(il->semname); - free(il->semname); - free(il); -} diff --git a/bin/xbps-rindex/sign.c b/bin/xbps-rindex/sign.c index 0e713d4d..b8fbecf8 100644 --- a/bin/xbps-rindex/sign.c +++ b/bin/xbps-rindex/sign.c @@ -118,7 +118,6 @@ int sign_repo(struct xbps_handle *xhp, const char *repodir, const char *privkey, const char *signedby) { - struct idxlock *il; struct stat st; struct xbps_repo *repo; xbps_dictionary_t pkgd, meta = NULL; @@ -139,13 +138,10 @@ sign_repo(struct xbps_handle *xhp, const char *repodir, return -1; } - if ((il = index_lock(xhp)) == NULL) - return EINVAL; - /* * Check that repository index exists and not empty, otherwise bail out. */ - repo = xbps_repo_open(xhp, repodir); + repo = xbps_repo_open(xhp, repodir, true); if (repo == NULL) { rv = errno; fprintf(stderr, "%s: cannot read repository data: %s\n", @@ -297,8 +293,6 @@ sign_repo(struct xbps_handle *xhp, const char *repodir, xbps_dictionary_count(repo->idx) == 1 ? "" : "s"); out: - index_unlock(il); - if (defprivkey) { free(defprivkey); } @@ -307,7 +301,7 @@ out: rsa = NULL; } if (repo) { - xbps_repo_close(repo); + xbps_repo_close(repo, true); } return rv ? -1 : 0; } diff --git a/configure b/configure index fe183b32..a3ed89f9 100755 --- a/configure +++ b/configure @@ -340,40 +340,6 @@ echo "CPPFLAGS += -I\$(TOPDIR)/lib/portableproplib/prop" >>$CONFIG_MK echo "LDFLAGS += -lpthread" >>$CONFIG_MK echo "STATIC_LIBS += -lpthread" >>$CONFIG_MK -# -# Check for POSIX semaphores. -# -printf "Checking for POSIX semaphores ... " -func=psem -cat < _$func.c -#include -#include -#include - -int main(void) { - sem_t *sem; - - sem = sem_open("/xbps0000test", 0644, O_CREAT, 1); - sem_wait(sem); - sem_post(sem); - sem_close(sem); - sem_unlink("/xbps0000test"); - - return 0; -} -EOF -if $XCC -pthread -lrt _$func.c -o _$func 2>/dev/null; then - POSIXSEM=yes - echo yes. -fi -rm -f _$func.c _$func -if [ -z "$POSIXSEM" ]; then - echo "no! POSIX semaphores are required, exiting..." - exit 1 -fi -echo "LDFLAGS += -lrt" >>$CONFIG_MK -echo "STATIC_LIBS += -lrt" >>$CONFIG_MK - # # Check for vasprintf(). # diff --git a/include/xbps.h.in b/include/xbps.h.in index d02610d6..154d61ea 100644 --- a/include/xbps.h.in +++ b/include/xbps.h.in @@ -48,7 +48,7 @@ * * This header documents the full API for the XBPS Library. */ -#define XBPS_API_VERSION "20140812" +#define XBPS_API_VERSION "20140905" #ifndef XBPS_VERSION #define XBPS_VERSION "UNSET" @@ -1245,6 +1245,10 @@ struct xbps_repo { * URI string associated with repository. */ const char *uri; + /** + * @private + */ + int fd; /** * var is_remote * @@ -1372,17 +1376,19 @@ xbps_dictionary_t xbps_rpool_get_pkg_plist(struct xbps_handle *xhp, * * @param[in] xhp Pointer to the xbps_handle struct. * @param[in] uri Repository URI to match. + * @param[in] lock Set it to true to acquire a POSIX file lock. * * @return The matching repository object, NULL otherwise. */ -struct xbps_repo *xbps_repo_open(struct xbps_handle *xhp, const char *url); +struct xbps_repo *xbps_repo_open(struct xbps_handle *xhp, const char *url, bool lock); /** * Closes a repository object and releases resources. * * @param[in] repo The repository object to close. + * @param[in] lock Set it to true to release the POSIX file lock. */ -void xbps_repo_close(struct xbps_repo *repo); +void xbps_repo_close(struct xbps_repo *repo, bool lock); /** diff --git a/lib/repo.c b/lib/repo.c index ef5e4d59..d0ad0cf4 100644 --- a/lib/repo.c +++ b/lib/repo.c @@ -107,7 +107,7 @@ repo_get_dict(struct xbps_repo *repo) } struct xbps_repo * -xbps_repo_open(struct xbps_handle *xhp, const char *url) +xbps_repo_open(struct xbps_handle *xhp, const char *url, bool lock) { struct xbps_repo *repo; struct stat st; @@ -148,34 +148,55 @@ xbps_repo_open(struct xbps_handle *xhp, const char *url) repofile, strerror(errno)); goto out; } + /* + * Open or create the repository archive. + */ + repo->fd = open(repofile, O_CREAT|O_RDWR, 0664); + if (repo->fd == -1) { + xbps_dbg_printf(xhp, "[repo] `%s' open repodata %s\n", + repofile, strerror(errno)); + goto out; + } + /* + * Acquire a POSIX file lock on the archive; wait if the lock is + * already taken. + */ + if (lock && lockf(repo->fd, F_LOCK, 0) == -1) { + xbps_dbg_printf(xhp, "[repo] failed to lock %s: %s\n", repo->uri, strerror(errno)); + goto out; + } repo->ar = archive_read_new(); archive_read_support_compression_gzip(repo->ar); archive_read_support_format_tar(repo->ar); - if (archive_read_open_filename(repo->ar, repofile, st.st_blksize) == ARCHIVE_FATAL) { + if (archive_read_open_fd(repo->ar, repo->fd, st.st_blksize) == ARCHIVE_FATAL) { xbps_dbg_printf(xhp, "[repo] `%s' failed to open repodata archive %s\n", repofile, strerror(archive_errno(repo->ar))); - archive_read_free(repo->ar); - repo->ar = NULL; goto out; } if ((repo->idx = repo_get_dict(repo)) == NULL) { xbps_dbg_printf(xhp, "[repo] `%s' failed to internalize index on archive %s: %s\n", url, repofile, strerror(archive_errno(repo->ar))); - archive_read_finish(repo->ar); - repo->ar = NULL; goto out; } repo->idxmeta = repo_get_dict(repo); if (repo->idxmeta != NULL) repo->is_signed = true; -out: free(repofile); return repo; + +out: + if (repo->ar) + archive_read_free(repo->ar); + if (repo->fd) + close(repo->fd); + free(repofile); + free(repo); + return NULL; } void @@ -186,7 +207,7 @@ xbps_repo_open_idxfiles(struct xbps_repo *repo) } void -xbps_repo_close(struct xbps_repo *repo) +xbps_repo_close(struct xbps_repo *repo, bool lock) { assert(repo); @@ -205,6 +226,11 @@ xbps_repo_close(struct xbps_repo *repo) xbps_object_release(repo->idxfiles); repo->idxfiles = NULL; } + if (lock && lockf(repo->fd, F_ULOCK, 0) == -1) + xbps_dbg_printf(repo->xhp, "[repo] failed to unlock %s: %s\n", repo->uri, strerror(errno)); + + close(repo->fd); + free(repo); } xbps_dictionary_t diff --git a/lib/rpool.c b/lib/rpool.c index 40ecc177..1a73c890 100644 --- a/lib/rpool.c +++ b/lib/rpool.c @@ -107,7 +107,7 @@ xbps_rpool_foreach(struct xbps_handle *xhp, xbps_array_get_cstring_nocopy(xhp->repositories, i, &repouri); repo = xbps_rpool_get_repo(repouri); if (!repo) { - repo = xbps_repo_open(xhp, repouri); + repo = xbps_repo_open(xhp, repouri, false); SIMPLEQ_INSERT_TAIL(&rpool_queue, repo, entries); xbps_dbg_printf(xhp, "[rpool] `%s' registered.\n", repouri); }