Use a POSIX lock for pkgdb and only issue pkgdb writes in exact points.

- Rather than using a POSIX named semaphore use a POSIX lock (lockf(3))
for pkgdb for writers. Writers that cannot acquire the pkgdb lock will
get EAGAIN rather then being blocked.

- Due to using a file lock we cannot write the pkgdb every time a package
is being unpacked, configured or removed. Instead pkgdb is only written
at the end of a specific point in the transaction (unpack, configure, remove)
or via xbps_pkgdb_unlock().

NEWS

@@ -8,6 +8,9 @@ xbps-0.33 (???):
    the output file will be named "bar.tar.gz". Contributed by
    Enno Boland (Gottox).
 
+ * Rather than performing a pkgdb write per package being unpacked, configured,
+   or removed, issue the write only once after the transaction phase has finished.
+
  * While checking file permissions for package files removal, make sure
    to not dereference symbolic links, because a symlink might point to
    a file in the rootfs, which won't have the same uid/gid.
@@ -20,7 +23,7 @@ xbps-0.33 (???):
 
  * Only allow a single writer to the pkgdb to avoid concurrency issues
    when performing multiple write transactions (install, update or remove).
-   The implementation relies on a POSIX named semaphore.
+   Processes that cannot acquire the lock will get an EAGAIN error.
 
  * Do not continue the transaction if downloading a binpkg/signature file
    has failed for some reason, stop and return error immediately.

bin/xbps-pkgdb/check.c

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2009-2013 Juan Romero Pardines.
+ * Copyright (c) 2009-2014 Juan Romero Pardines.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -64,19 +64,7 @@ pkgdb_cb(struct xbps_handle *xhp _unused,
 int
 check_pkg_integrity_all(struct xbps_handle *xhp)
 {
-	int rv;
+	return xbps_pkgdb_foreach_cb_multi(xhp, pkgdb_cb, NULL);
-
-	/* force an update to get total pkg count */
-	(void)xbps_pkgdb_update(xhp, false);
-
-	rv = xbps_pkgdb_foreach_cb_multi(xhp, pkgdb_cb, NULL);
-
-	if ((rv = xbps_pkgdb_update(xhp, true)) != 0) {
-		xbps_error_printf("failed to write pkgdb: %s\n",
-		    strerror(rv));
-		return rv;
-	}
-	return 0;
 }
 
 int
@@ -151,8 +139,5 @@ do {								\
 
 #undef RUN_PKG_CHECK
 
-	if ((rv == 0) && (pkgd == NULL))
-		(void)xbps_pkgdb_update(xhp, true);
-
 	return 0;
 }

bin/xbps-pkgdb/main.c

@@ -72,7 +72,7 @@ change_pkg_mode(struct xbps_handle *xhp, const char *pkgname, const char *mode)
 	else
 		usage(true);
 
-	return xbps_pkgdb_update(xhp, true);
+	return 0;
 }
 
 int

bin/xbps-reconfigure/main.c

@@ -153,17 +153,22 @@ main(int argc, char **argv)
 		exit(EXIT_FAILURE);
 	}
 
-	if (all) {
+	if ((rv = xbps_pkgdb_lock(&xh)) != 0) {
-		rv = xbps_configure_packages(&xh, true);
+		fprintf(stderr, "failed to lock pkgdb: %s\n", strerror(rv));
-	} else {
+		exit(EXIT_FAILURE);
-		for (i = optind; i < argc; i++) {
-			rv = xbps_configure_pkg(&xh, argv[i],
-			    true, false, true);
-			if (rv != 0)
-				fprintf(stderr, "Failed to reconfigure "
-				    "`%s': %s\n", argv[i], strerror(rv));
-		}
 	}
 
+	if (all) {
+		rv = xbps_configure_packages(&xh);
+	} else {
+		for (i = optind; i < argc; i++) {
+			rv = xbps_configure_pkg(&xh, argv[i], true, false);
+			if (rv != 0) {
+				fprintf(stderr, "Failed to reconfigure "
+				    "`%s': %s\n", argv[i], strerror(rv));
+			}
+		}
+	}
+	xbps_pkgdb_unlock(&xh);
 	exit(rv ? EXIT_FAILURE : EXIT_SUCCESS);
 }

include/xbps.h.in

@@ -29,8 +29,6 @@
 
 #include <stdio.h>
 #include <inttypes.h>
-#include <limits.h>
-#include <semaphore.h>
 
 #include <xbps/xbps_array.h>
 #include <xbps/xbps_bool.h>
@@ -50,7 +48,7 @@
  *
  * This header documents the full API for the XBPS Library.
  */
-#define XBPS_API_VERSION	"20140225"
+#define XBPS_API_VERSION	"20140304"
 
 #ifndef XBPS_VERSION
  #define XBPS_VERSION		"UNSET"
@@ -471,11 +469,11 @@ struct xbps_handle {
 	 */
 	xbps_dictionary_t pkgdb;
 	/**
-	 * @private
+	 * @var pkgdb_plist;
 	 *
-	 * POSIX named semaphore associated with the pkgdb for writers.
+	 * Absolute pathname to the pkgdb plist file.
 	 */
-	sem_t *pkgdb_sem;
+	char *pkgdb_plist;
 	/**
 	 * @var transd
 	 *
@@ -632,25 +630,20 @@ void xbps_end(struct xbps_handle *xhp);
  * @param[in] check_state Set it to true to check that package is
  * in unpacked state.
  * @param[in] update Set it to true if this package is being updated.
- * @param[in] flush Set it to true to flush state to pkgdb.
  *
  * @return 0 on success, otherwise an errno value.
  */
-int xbps_configure_pkg(struct xbps_handle *xhp,
+int xbps_configure_pkg(struct xbps_handle *xhp, const char *pkgname,
-		       const char *pkgname,
+		bool check_state, bool update);
-		       bool check_state,
-		       bool update,
-		       bool flush);
 
 /**
  * Configure (or force reconfiguration of) all packages.
  *
  * @param[in] xhp Pointer to an xbps_handle struct.
- * @param[in] flush Set it to true to flush state to pkgdb.
  *
  * @return 0 on success, otherwise an errno value.
  */
-int xbps_configure_packages(struct xbps_handle *xhp, bool flush);
+int xbps_configure_packages(struct xbps_handle *xhp);
 
 /*@}*/
 

lib/initend.c

@@ -368,10 +368,6 @@ xbps_end(struct xbps_handle *xhp)
 	assert(xhp);
 
 	xbps_pkgdb_release(xhp);
-
-	if (xbps_object_type(xhp->pkgdb_revdeps) != XBPS_TYPE_UNKNOWN)
-		xbps_object_release(xhp->pkgdb_revdeps);
-
 	xbps_fetch_unset_cache_connection();
 }
 

lib/package_configure.c

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2009-2013 Juan Romero Pardines.
+ * Copyright (c) 2009-2014 Juan Romero Pardines.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -46,7 +46,7 @@
  * state is XBPS_PKG_STATE_INSTALLED.
  */
 int
-xbps_configure_packages(struct xbps_handle *xhp, bool flush)
+xbps_configure_packages(struct xbps_handle *xhp)
 {
 	xbps_dictionary_t pkgd;
 	xbps_object_t obj;
@@ -62,7 +62,7 @@ xbps_configure_packages(struct xbps_handle *xhp, bool flush)
 	while ((obj = xbps_object_iterator_next(iter))) {
 		pkgd = xbps_dictionary_get_keysym(xhp->pkgdb, obj);
 		xbps_dictionary_get_cstring_nocopy(pkgd, "pkgver", &pkgver);
-		rv = xbps_configure_pkg(xhp, pkgver, true, false, false);
+		rv = xbps_configure_pkg(xhp, pkgver, true, false);
 		if (rv != 0) {
 			xbps_dbg_printf(xhp, "%s: failed to configure %s: %s\n",
 			    __func__, pkgver, strerror(rv));
@@ -71,9 +71,6 @@ xbps_configure_packages(struct xbps_handle *xhp, bool flush)
 	}
 	xbps_object_iterator_release(iter);
 
-	if ((rv == 0) && flush)
-		rv = xbps_pkgdb_update(xhp, true);
-
 	return rv;
 }
 
@@ -81,8 +78,7 @@ int
 xbps_configure_pkg(struct xbps_handle *xhp,
 		   const char *pkgver,
 		   bool check_state,
-		   bool update,
+		   bool update)
-		   bool flush)
 {
 	xbps_dictionary_t pkgd, pkgmetad;
 	char *pkgname, *plist;
@@ -154,13 +150,6 @@ xbps_configure_pkg(struct xbps_handle *xhp,
 		    pkgver, strerror(rv));
 		return rv;
 	}
-	if (flush) {
-		if ((rv = xbps_pkgdb_update(xhp, true)) != 0) {
-			xbps_set_cb_state(xhp, XBPS_STATE_CONFIGURE_FAIL, rv,
-			    pkgver, "%s: [configure] failed to update pkgdb: %s\n",
-			    pkgver, strerror(rv));
-		}
-	}
 	xbps_object_release(pkgmetad);
 
 	if (rv == 0)

lib/package_register.c

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2008-2013 Juan Romero Pardines.
+ * Copyright (c) 2008-2014 Juan Romero Pardines.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -140,10 +140,7 @@ xbps_register_pkg(struct xbps_handle *xhp, xbps_dictionary_t pkgrd)
 	if (!xbps_dictionary_set(xhp->pkgdb, pkgname, pkgd)) {
 		xbps_dbg_printf(xhp,
 		    "%s: failed to set pkgd for %s\n", __func__, pkgver);
-		goto out;
 	}
-	rv = xbps_pkgdb_update(xhp, true);
-
 out:
 	if (pkgname)
 		free(pkgname);

lib/package_remove.c

@@ -370,11 +370,7 @@ purge:
 	 * Unregister package from pkgdb.
 	 */
 	xbps_dictionary_remove(xhp->pkgdb, pkgname);
-	if ((rv = xbps_pkgdb_update(xhp, true)) != 0)
-		goto out;
-
 	xbps_dbg_printf(xhp, "[remove] unregister %s returned %d\n", pkgver, rv);
-
 	xbps_set_cb_state(xhp, XBPS_STATE_REMOVE_DONE, 0, pkgver, NULL);
 out:
 	if (pkgname != NULL)

lib/pkgdb.c

@@ -57,29 +57,49 @@
  * data type is specified on its edge, i.e array, bool, integer, string,
  * dictionary.
  */
-#define _PKGDB_SEMNAME	"/libxbps-pkgdb"
+static int pkgdb_fd;
+
 int
 xbps_pkgdb_lock(struct xbps_handle *xhp)
 {
-	int rv = 0;
+	int rv;
-	mode_t myumask;
 	/*
-	 * Create/open the POSIX named semaphore.
+	 * Use a mandatory file lock to only allow one writer to pkgdb,
+	 * other writers will block.
 	 */
-	myumask = umask(0);
+	xhp->pkgdb_plist = xbps_xasprintf("%s/%s", xhp->metadir, XBPS_PKGDB);
-	xhp->pkgdb_sem = sem_open(_PKGDB_SEMNAME, O_CREAT, 0660, 1);
+	if (xbps_pkgdb_init(xhp) == ENOENT) {
-	umask(myumask);
+		/* if metadir does not exist, create it */
+		if (access(xhp->metadir, R_OK|X_OK) == -1) {
+			if (errno != ENOENT)
+				return errno;
 
-	if (xhp->pkgdb_sem == SEM_FAILED) {
+			if (xbps_mkpath(xhp->metadir, 0755) == -1) {
+				rv = errno;
+				xbps_dbg_printf(xhp, "[pkgdb] failed to create metadir "
+				    "%s: %s\n", xhp->metadir, strerror(rv));
+				return rv;
+			}
+		}
+		/* if pkgdb is unexistent, create it with an empty dictionary */
+		xhp->pkgdb = xbps_dictionary_create();
+		if (!xbps_dictionary_externalize_to_file(xhp->pkgdb, xhp->pkgdb_plist)) {
+			rv = errno;
+			xbps_dbg_printf(xhp, "[pkgdb] failed to create pkgdb "
+			    "%s: %s\n", xhp->pkgdb_plist, strerror(rv));
+			return rv;
+		}
+	}
+	if ((pkgdb_fd = open(xhp->pkgdb_plist, O_CREAT|O_RDWR, 0664)) == -1) {
 		rv = errno;
-		xbps_dbg_printf(xhp, "pkgdb: failed to create/open named "
+		xbps_dbg_printf(xhp, "[pkgdb] cannot open pkgdb for locking "
-		    "semaphore: %s\n", strerror(errno));
+		    "%s: %s\n", xhp->pkgdb_plist, strerror(rv));
+		free(xhp->pkgdb_plist);
 		return rv;
 	}
-	if (sem_wait(xhp->pkgdb_sem) == -1) {
+	if (lockf(pkgdb_fd, F_TLOCK, 0) == -1) {
 		rv = errno;
-		xbps_dbg_printf(xhp, "pkgdb: failed to lock named semaphore: %s\n",
+		xbps_dbg_printf(xhp, "[pkgdb] cannot lock pkgdb: %s\n", strerror(rv));
-		    strerror(errno));
 		return rv;
 	}
 	return 0;
@@ -88,16 +108,14 @@ xbps_pkgdb_lock(struct xbps_handle *xhp)
 void
 xbps_pkgdb_unlock(struct xbps_handle *xhp)
 {
-	/* Unlock semaphore, close and destroy it (if possible) */
+	(void)xbps_pkgdb_update(xhp, true);
-	if (xhp->pkgdb_sem == NULL)
-		return;
 
-	sem_post(xhp->pkgdb_sem);
+	if (lockf(pkgdb_fd, F_ULOCK, 0) == -1)
-	sem_close(xhp->pkgdb_sem);
+		xbps_dbg_printf(xhp, "[pkgdb] failed to unlock pkgdb: %s\n", strerror(errno));
-	sem_unlink(_PKGDB_SEMNAME);
+
-	xhp->pkgdb_sem = NULL;
+	(void)close(pkgdb_fd);
+	pkgdb_fd = -1;
 }
-#undef _PKGDB_SEMNAME
 
 int HIDDEN
 xbps_pkgdb_init(struct xbps_handle *xhp)
@@ -125,23 +143,19 @@ int
 xbps_pkgdb_update(struct xbps_handle *xhp, bool flush)
 {
 	xbps_dictionary_t pkgdb_storage;
-	char *plist;
 	static int cached_rv;
 	int rv = 0;
 
 	if (cached_rv && !flush)
 		return cached_rv;
 
-	plist = xbps_xasprintf("%s/%s", xhp->metadir, XBPS_PKGDB);
 	if (xhp->pkgdb && flush) {
-		pkgdb_storage = xbps_dictionary_internalize_from_file(plist);
+		pkgdb_storage = xbps_dictionary_internalize_from_file(xhp->pkgdb_plist);
 		if (pkgdb_storage == NULL ||
 		    !xbps_dictionary_equals(xhp->pkgdb, pkgdb_storage)) {
 			/* flush dictionary to storage */
-			if (!xbps_dictionary_externalize_to_file(xhp->pkgdb, plist)) {
+			if (!xbps_dictionary_externalize_to_file(xhp->pkgdb, xhp->pkgdb_plist))
-				free(plist);
 				return errno;
-			}
 		}
 		if (pkgdb_storage)
 			xbps_object_release(pkgdb_storage);
@@ -151,7 +165,7 @@ xbps_pkgdb_update(struct xbps_handle *xhp, bool flush)
 		cached_rv = 0;
 	}
 	/* update copy in memory */
-	if ((xhp->pkgdb = xbps_dictionary_internalize_from_file(plist)) == NULL) {
+	if ((xhp->pkgdb = xbps_dictionary_internalize_from_file(xhp->pkgdb_plist)) == NULL) {
 		if (errno == ENOENT)
 			xhp->pkgdb = xbps_dictionary_create();
 		else
@@ -159,7 +173,6 @@ xbps_pkgdb_update(struct xbps_handle *xhp, bool flush)
 
 		cached_rv = rv = errno;
 	}
-	free(plist);
 
 	return rv;
 }
@@ -172,6 +185,9 @@ xbps_pkgdb_release(struct xbps_handle *xhp)
 	if (xhp->pkgdb == NULL)
 		return;
 
+	/* write pkgdb to storage in case it was modified */
+	(void)xbps_pkgdb_update(xhp, true);
+
 	if (xbps_object_type(xhp->pkg_metad) == XBPS_TYPE_DICTIONARY)
 	       xbps_object_release(xhp->pkg_metad);
 

lib/transaction_commit.c

@@ -265,7 +265,7 @@ xbps_transaction_commit(struct xbps_handle *xhp)
 			/*
 			 * Reconfigure pending package.
 			 */
-			rv = xbps_configure_pkg(xhp, pkgver, false, false, false);
+			rv = xbps_configure_pkg(xhp, pkgver, false, false);
 			if (rv != 0)
 				goto out;
 
@@ -313,6 +313,8 @@ xbps_transaction_commit(struct xbps_handle *xhp)
 		goto out;
 
 	xbps_object_iterator_reset(iter);
+	/* Force a pkgdb write for all unpacked pkgs in transaction */
+	(void)xbps_pkgdb_update(xhp, true);
 
 	/*
 	 * Configure all unpacked packages.
@@ -330,7 +332,7 @@ xbps_transaction_commit(struct xbps_handle *xhp)
 		if (strcmp(tract, "update") == 0)
 			update = true;
 
-		rv = xbps_configure_pkg(xhp, pkgver, false, update, true);
+		rv = xbps_configure_pkg(xhp, pkgver, false, update);
 		if (rv != 0) {
 			xbps_dbg_printf(xhp, "%s: configure failed for "
 			    "%s: %s\n", __func__, pkgver, strerror(rv));
@@ -351,6 +353,8 @@ xbps_transaction_commit(struct xbps_handle *xhp)
 
 out:
 	xbps_object_iterator_release(iter);
+	/* Force a pkgdb write for all unpacked pkgs in transaction */
+	(void)xbps_pkgdb_update(xhp, true);
 
 	return rv;
 }