From 4d1cdcac0cee5e43706cc367d64ce3656a90213d Mon Sep 17 00:00:00 2001 From: Juan RP Date: Sun, 23 Feb 2014 08:23:14 +0100 Subject: [PATCH] Fix concurrency issues in pkgdb: only allow 1 write transaction at the same time. This implementation relies on a POSIX named semaphore, which is also required by xbps-rindex(8). --- NEWS | 4 ++++ README.md | 1 + TODO | 1 - bin/xbps-install/main.c | 16 +++++++++++--- bin/xbps-pkgdb/main.c | 9 +++++++- bin/xbps-remove/main.c | 16 ++++++++++---- include/xbps.h.in | 35 +++++++++++++++++++++++++----- include/xbps_api_impl.h | 2 +- lib/pkgdb.c | 47 ++++++++++++++++++++++++++++++++++++++++- 9 files changed, 115 insertions(+), 16 deletions(-) diff --git a/NEWS b/NEWS index c4bcb60a..8d4721c3 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,9 @@ 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. + * Do not continue the transaction if downloading a binpkg/signature file has failed for some reason, stop and return error immediately. diff --git a/README.md b/README.md index e259efd5..49a790b4 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ 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/TODO b/TODO index 21627227..1d94a688 100644 --- a/TODO +++ b/TODO @@ -2,7 +2,6 @@ libxbps: - transaction: avoid fetching the whole pkg when updating and only fetch modified files from target pkg. - Add support to handle 'shlib-requires' and 'shlib-provides'. - - Concurrency issues with pkgdb. xbps-create: - Move all configuration files to /share//conf/. diff --git a/bin/xbps-install/main.c b/bin/xbps-install/main.c index 2b010933..dd03ef19 100644 --- a/bin/xbps-install/main.c +++ b/bin/xbps-install/main.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 @@ -216,6 +216,11 @@ main(int argc, char **argv) if (sync && !update && (argc == optind)) exit(EXIT_SUCCESS); + if ((rv = xbps_pkgdb_lock(&xh)) != 0) { + fprintf(stderr, "Failed to lock the pkgdb: %s\n", strerror(rv)); + exit(rv); + } + if (update && (argc == optind)) { /* Update all installed packages */ rv = dist_upgrade(&xh, maxcols, yes, drun); @@ -223,19 +228,24 @@ main(int argc, char **argv) /* Update target packages */ for (i = optind; i < argc; i++) { rv = update_pkg(&xh, argv[i]); - if (rv != 0) + if (rv != 0) { + xbps_pkgdb_unlock(&xh); exit(rv); + } } rv = exec_transaction(&xh, maxcols, yes, drun); } else if (!update) { /* Install target packages */ for (i = optind; i < argc; i++) { rv = install_new_pkg(&xh, argv[i], reinstall); - if (rv != 0) + if (rv != 0) { + xbps_pkgdb_unlock(&xh); exit(rv); + } } rv = exec_transaction(&xh, maxcols, yes, drun); } + xbps_pkgdb_unlock(&xh); exit(rv); } diff --git a/bin/xbps-pkgdb/main.c b/bin/xbps-pkgdb/main.c index 8b5602b1..992f980e 100644 --- a/bin/xbps-pkgdb/main.c +++ b/bin/xbps-pkgdb/main.c @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2013 Juan Romero Pardines. + * Copyright (c) 2013-2014 Juan Romero Pardines. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -146,12 +146,17 @@ main(int argc, char **argv) exit(EXIT_FAILURE); } + if ((rv = xbps_pkgdb_lock(&xh)) != 0) { + fprintf(stderr, "failed to lock pkgdb: %s\n", strerror(rv)); + exit(EXIT_FAILURE); + } if (update_format) convert_pkgdb_format(&xh); else if (instmode) { if (argc == optind) { fprintf(stderr, "xbps-pkgdb: missing PKGNAME argument\n"); + xbps_pkgdb_unlock(&xh); exit(EXIT_FAILURE); } for (i = optind; i < argc; i++) { @@ -160,6 +165,7 @@ main(int argc, char **argv) fprintf(stderr, "xbps-pkgdb: failed to " "change to %s mode to %s: %s\n", instmode, argv[i], strerror(rv)); + xbps_pkgdb_unlock(&xh); exit(EXIT_FAILURE); } } @@ -174,5 +180,6 @@ main(int argc, char **argv) } } + xbps_pkgdb_unlock(&xh); exit(rv ? EXIT_FAILURE : EXIT_SUCCESS); } diff --git a/bin/xbps-remove/main.c b/bin/xbps-remove/main.c index e61593ef..1c995114 100644 --- a/bin/xbps-remove/main.c +++ b/bin/xbps-remove/main.c @@ -350,8 +350,14 @@ main(int argc, char **argv) exit(rv);; } + if ((rv = xbps_pkgdb_lock(&xh)) != 0) { + fprintf(stderr, "failed to lock pkgdb: %s\n", strerror(rv)); + exit(rv); + } + if (orphans) { if ((rv = xbps_transaction_autoremove_pkgs(&xh)) != 0) { + xbps_pkgdb_unlock(&xh); if (rv != ENOENT) { fprintf(stderr, "Failed to queue package " "orphans: %s\n", strerror(rv)); @@ -370,11 +376,13 @@ main(int argc, char **argv) else reqby_force = true; } - if (reqby_force && !ignore_revdeps) + if (reqby_force && !ignore_revdeps) { + xbps_pkgdb_unlock(&xh); exit(EXIT_FAILURE); - - if (orphans || (argc > optind)) + } + if (orphans || (argc > optind)) { rv = exec_transaction(&xh, maxcols, yes, drun); - + } + xbps_pkgdb_unlock(&xh); exit(rv); } diff --git a/include/xbps.h.in b/include/xbps.h.in index 090fccd5..a53e0fc9 100644 --- a/include/xbps.h.in +++ b/include/xbps.h.in @@ -24,13 +24,13 @@ *- */ -#ifndef _XBPS_API_H_ -#define _XBPS_API_H_ - +#ifndef _XBPS_H_ +#define _XBPS_H_ #include #include #include +#include #include #include @@ -50,7 +50,7 @@ * * This header documents the full API for the XBPS Library. */ -#define XBPS_API_VERSION "20140130" +#define XBPS_API_VERSION "20140223" #ifndef XBPS_VERSION #define XBPS_VERSION "UNSET" @@ -475,6 +475,12 @@ struct xbps_handle { * stored in XBPS_META_PATH/XBPS_PKGDB. */ xbps_dictionary_t pkgdb; + /** + * @private + * + * POSIX named semaphore associated with the pkgdb for writers. + */ + sem_t *pkgdb_sem; /** * @var transd * @@ -706,6 +712,25 @@ xbps_array_t xbps_find_pkg_obsoletes(struct xbps_handle *xhp, /** @addtogroup pkgdb */ /*@{*/ +/** + * Locks the pkgdb to allow a write transaction. + * + * This routine should be called before a write transaction is the target: + * install, remove or update. + * + * @param[in] xhp The pointer to the xbps_handle struct. + * @return 0 on success, otherwise an errno value. + */ +int xbps_pkgdb_lock(struct xbps_handle *); + +/** + * Unlocks the pkgdb after a write transaction: the POSIX named semaphore + * is unlocked and unlinked. + * + * @param[in] xhp The pointer to the xbps_handle struct. + */ +void xbps_pkgdb_unlock(struct xbps_handle *); + /** * Executes a function callback per a package dictionary registered * in master package database (pkgdb) plist. @@ -1693,4 +1718,4 @@ char *xbps_pubkey2fp(struct xbps_handle *xhp, xbps_data_t pubkey); } #endif -#endif /* !_XBPS_API_H_ */ +#endif /* !_XBPS_H_ */ diff --git a/include/xbps_api_impl.h b/include/xbps_api_impl.h index b1eecddc..237c8ea4 100644 --- a/include/xbps_api_impl.h +++ b/include/xbps_api_impl.h @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2010-2013 Juan Romero Pardines. + * Copyright (c) 2010-2014 Juan Romero Pardines. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/lib/pkgdb.c b/lib/pkgdb.c index 10146d8a..d2311f24 100644 --- a/lib/pkgdb.c +++ b/lib/pkgdb.c @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2012-2013 Juan Romero Pardines. + * Copyright (c) 2012-2014 Juan Romero Pardines. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -28,6 +28,9 @@ #include #include #include +#include +#include +#include #include "xbps_api_impl.h" @@ -54,6 +57,48 @@ * data type is specified on its edge, i.e array, bool, integer, string, * dictionary. */ +#define _PKGDB_SEMNAME "/libxbps-pkgdb" +int +xbps_pkgdb_lock(struct xbps_handle *xhp) +{ + int rv = 0; + mode_t myumask; + /* + * Create/open the POSIX named semaphore. + */ + myumask = umask(0); + xhp->pkgdb_sem = sem_open(_PKGDB_SEMNAME, O_CREAT, 0660, 1); + umask(myumask); + + if (xhp->pkgdb_sem == SEM_FAILED) { + rv = errno; + xbps_dbg_printf(xhp, "pkgdb: failed to create/open named " + "semaphore: %s\n", strerror(errno)); + return rv; + } + if (sem_wait(xhp->pkgdb_sem) == -1) { + rv = errno; + xbps_dbg_printf(xhp, "pkgdb: failed to lock named semaphore: %s\n", + strerror(errno)); + return rv; + } + return 0; +} + +void +xbps_pkgdb_unlock(struct xbps_handle *xhp) +{ + /* Unlock semaphore, close and destroy it (if possible) */ + if (xhp->pkgdb_sem == NULL) + return; + + sem_post(xhp->pkgdb_sem); + sem_close(xhp->pkgdb_sem); + sem_unlink(_PKGDB_SEMNAME); + xhp->pkgdb_sem = NULL; +} +#undef _PKGDB_SEMNAME + int HIDDEN xbps_pkgdb_init(struct xbps_handle *xhp) {