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

@@ -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!
 

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)

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

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_ */

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;
 }

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)

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;

bin/xbps-rindex/sem.c

@@ -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 <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <errno.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-
-#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);
-}

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;
 }

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 <<EOF > _$func.c
-#include <fcntl.h>
-#include <sys/stat.h>
-#include <semaphore.h>
-
-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().
 #

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);
 
 
 /**

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

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);
 		}