[pacman-dev] [PATCH 1/1] alpm: use flock() for db lock
From: Christian Hesse <mail@eworm.de> We used to check for file existens, but that suffers from stale lock files caused by unexpected events like crash, shutdown, etc. Instead use flock() to lock the file. It does not matter whether or not the file exists but whether an exclusive lock can be obtained. Also remove the hint about removing the file from pacman. Signed-off-by: Christian Hesse <mail@eworm.de> --- lib/libalpm/handle.c | 17 +++++++++++++++-- src/pacman/util.c | 5 ----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index b6b27881..bb5cc907 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -24,6 +24,7 @@ #include <stdlib.h> #include <string.h> #include <limits.h> +#include <sys/file.h> #include <sys/types.h> #include <syslog.h> #include <sys/stat.h> @@ -120,10 +121,20 @@ int _alpm_handle_lock(alpm_handle_t *handle) FREE(dir); do { - handle->lockfd = open(handle->lockfile, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0000); + handle->lockfd = open(handle->lockfile, O_WRONLY | O_CREAT | O_CLOEXEC, 0000); } while(handle->lockfd == -1 && errno == EINTR); - return (handle->lockfd >= 0 ? 0 : -1); + if (handle->lockfd == -1) + return -1; + + /* try to get an exclusive lock */ + if (flock(handle->lockfd, LOCK_EX | LOCK_NB) == -1) { + close(handle->lockfd); + handle->lockfd = -1; + return -1; + } + + return 0; } /** Remove the database lock file @@ -137,6 +148,8 @@ int SYMEXPORT alpm_unlock(alpm_handle_t *handle) ASSERT(handle->lockfile != NULL, return 0); ASSERT(handle->lockfd >= 0, return 0); + flock(handle->lockfd, LOCK_UN); + close(handle->lockfd); handle->lockfd = -1; diff --git a/src/pacman/util.c b/src/pacman/util.c index ae8a74d3..c3f9d3ba 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -81,13 +81,8 @@ void trans_init_error(void) pm_printf(ALPM_LOG_ERROR, _("failed to init transaction (%s)\n"), alpm_strerror(err)); if(err == ALPM_ERR_HANDLE_LOCK) { - const char *lockfile = alpm_option_get_lockfile(config->handle); pm_printf(ALPM_LOG_ERROR, _("could not lock database: %s\n"), strerror(errno)); - if(access(lockfile, F_OK) == 0) { - fprintf(stderr, _(" if you're sure a package manager is not already\n" - " running, you can remove %s\n"), lockfile); - } } } -- 2.13.1
On 06/06/17 at 11:26pm, Christian Hesse wrote:
From: Christian Hesse <mail@eworm.de>
We used to check for file existens, but that suffers from stale lock files caused by unexpected events like crash, shutdown, etc.
Instead use flock() to lock the file. It does not matter whether or not the file exists but whether an exclusive lock can be obtained.
Also remove the hint about removing the file from pacman.
Signed-off-by: Christian Hesse <mail@eworm.de> --- lib/libalpm/handle.c | 17 +++++++++++++++-- src/pacman/util.c | 5 ----- 2 files changed, 15 insertions(+), 7 deletions(-)
Refusing to run when a lock file is leftover from a previous invocation is intentional. It serves as an indicator that the user needs to verify the integrity of their system. Also, see https://lists.archlinux.org/pipermail/pacman-dev/2013-August/017733.html for previous discussion regarding flock(). apg
Andrew Gregory <andrew.gregory.8@gmail.com> on Tue, 2017/06/06 17:56:
On 06/06/17 at 11:26pm, Christian Hesse wrote:
From: Christian Hesse <mail@eworm.de>
We used to check for file existens, but that suffers from stale lock files caused by unexpected events like crash, shutdown, etc.
Instead use flock() to lock the file. It does not matter whether or not the file exists but whether an exclusive lock can be obtained.
Also remove the hint about removing the file from pacman.
Signed-off-by: Christian Hesse <mail@eworm.de> --- lib/libalpm/handle.c | 17 +++++++++++++++-- src/pacman/util.c | 5 ----- 2 files changed, 15 insertions(+), 7 deletions(-)
Refusing to run when a lock file is leftover from a previous invocation is intentional. It serves as an indicator that the user needs to verify the integrity of their system. Also, see https://lists.archlinux.org/pipermail/pacman-dev/2013-August/017733.html for previous discussion regarding flock().
Thanks for the heads-up. I did not know this thread. Looks like I should invest more time searching for this kind of information before reinventing the wheel. Perhaps anybody should add a comment to lib/libalpm/handle.c why we are *not* adding flock(). ;) BTW, it is interesting what systems actually do run pacman... -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
participants (2)
-
Andrew Gregory
-
Christian Hesse