[pacman-dev] [PATCH] Use file locks (flock) to stop multiple pacman instances from running concurrently. This improves the current model by automatically removing locks held by programs has failed to remove the lock, for example at a crash, forced kill or power outage.
Dave Reisner
d at falconindy.com
Wed Aug 14 21:26:54 EDT 2013
On Aug 14, 2013 8:38 PM, "Mattias Andrée" <maandree at member.fsf.org> wrote:
>
> I understood that. But I did not know that pacman supports
> BSD or Max OS X, nor that it was not portable. Just leaving
> it out because of NFS seems silly, it is not up to the
> program to account for all software that does not implement
> expected standards.
Uhhh but it is. That's sort of the idea of software portability. And since
when is file locking support anything close to standardized? It's
incredibly OS and filesystem specific.
>
>
> On Wed, 14 Aug 2013 19:29:41 -0400
> Dave Reisner <d at falconindy.com> wrote:
>
> > On Aug 14, 2013 7:21 PM, "Mattias Andrée"
> > <maandree at member.fsf.org> wrote:
> > >
> > > That is ridiculus that NFS does not implement flock, but
> > > there is a solution documented in flock(2):
> > >
> > > flock() does not lock files over NFS. Use fcntl(2)
> > > instead: that does work over NFS, given a sufficiently
> > > recent version of Linux and a server which supports
> > > locking.
> >
> > Keyword being Linux. We support BSD and OSX as well.
> > fcntl semantics aren't portable.
> >
> > Again, you're not the first to go down this road.
> >
> > >
> > >
> > > On Wed, 14 Aug 2013 18:14:20 -0400
> > > Dave Reisner <d at falconindy.com> wrote:
> > >
> > > > On Thu, Aug 15, 2013 at 12:03:33AM +0200, Mattias
> > > > Andrée wrote:
> > > > > This allows for further improvement of pacman where
> > > > > shared locks could also be used, in cases there
> > > > > pacman only reads the database and does not edit it.
> > > > >
> > > > > File locks also makes it easy to let pacman wait
> > > > > until the block instance has finished, by just not
> > > > > using the LOCK_NB flag. ---
> > > >
> > > > And the question that comes up every time this patch
> > > > arises: what about NFS?
> > > >
> > > > No, we won't change this.
> > > >
> > > > > lib/libalpm/be_sync.c | 5 +----
> > > > > lib/libalpm/handle.c | 26
> > > > > ++++++++++++++------------
> > > > > lib/libalpm/handle.h | 2 +-
> > > > > lib/libalpm/trans.c | 7 +------
> > > > > scripts/pacman-db-upgrade.sh.in | 9 +++++----
> > > > > scripts/pacman-optimize.sh.in | 9 +++++---- 6
> > > > > files changed, 27 insertions(+), 31 deletions(-)
> > > > >
> > > > > diff --git a/lib/libalpm/be_sync.c
> > > > > b/lib/libalpm/be_sync.c index 0b99684..e7c8c4b
> > > > > 100644 --- a/lib/libalpm/be_sync.c
> > > > > +++ b/lib/libalpm/be_sync.c
> > > > > @@ -284,10 +284,7 @@ int SYMEXPORT
> > > > > alpm_db_update(int force, alpm_db_t *db)
> > > > > cleanup:
> > > > >
> > > > > - if(_alpm_handle_unlock(handle)) {
> > > > > - _alpm_log(handle, ALPM_LOG_WARNING,
> > > > > _("could not remove lock file %s\n"),
> > > > > - handle->lockfile);
> > > > > - }
> > > > > + _alpm_handle_unlock(handle);
> > > > > free(syncpath);
> > > > > umask(oldmask);
> > > > > return ret;
> > > > > diff --git a/lib/libalpm/handle.c
> > > > > b/lib/libalpm/handle.c index 53c86c5..5d1cfb7 100644
> > > > > --- a/lib/libalpm/handle.c
> > > > > +++ b/lib/libalpm/handle.c
> > > > > @@ -27,6 +27,7 @@
> > > > > #include <sys/types.h>
> > > > > #include <syslog.h>
> > > > > #include <sys/stat.h>
> > > > > +#include <sys/file.h>
> > > > > #include <fcntl.h>
> > > > >
> > > > > /* libalpm */
> > > > > @@ -44,6 +45,7 @@ alpm_handle_t
> > > > > *_alpm_handle_new(void)
> > > > >
> > > > > CALLOC(handle, 1, sizeof(alpm_handle_t),
> > > > > return NULL); handle->deltaratio = 0.0;
> > > > > + handle->lckstream = -1;
> > > > >
> > > > > return handle;
> > > > > }
> > > > > @@ -95,7 +97,7 @@ int
> > > > > _alpm_handle_lock(alpm_handle_t *handle) char *dir,
> > > > > *ptr;
> > > > >
> > > > > ASSERT(handle->lockfile != NULL, return -1);
> > > > > - ASSERT(handle->lckstream == NULL, return 0);
> > > > > + ASSERT(handle->lckstream < 0, return 0);
> > > > >
> > > > > /* create the dir of the lockfile first */
> > > > > dir = strdup(handle->lockfile);
> > > > > @@ -110,14 +112,14 @@ int
> > > > > _alpm_handle_lock(alpm_handle_t *handle) FREE(dir);
> > > > >
> > > > > do {
> > > > > - fd = open(handle->lockfile, O_WRONLY |
> > > > > O_CREAT | O_EXCL, 0000);
> > > > > + fd = open(handle->lockfile, O_WRONLY |
> > > > > O_CREAT); } while(fd == -1 && errno == EINTR);
> > > > > if(fd >= 0) {
> > > > > - FILE *f = fdopen(fd, "w");
> > > > > - fprintf(f, "%ld\n", (long)getpid());
> > > > > - fflush(f);
> > > > > fsync(fd);
> > > > > - handle->lckstream = f;
> > > > > + if(flock(fd, LOCK_EX | LOCK_NB) < 0) {
> > > > > + return -1;
> > > > > + }
> > > > > + handle->lckstream = fd;
> > > > > return 0;
> > > > > }
> > > > > return -1;
> > > > > @@ -127,14 +129,14 @@ int
> > > > > _alpm_handle_lock(alpm_handle_t *handle) int
> > > > > _alpm_handle_unlock(alpm_handle_t *handle) {
> > > > > ASSERT(handle->lockfile != NULL, return -1);
> > > > > - ASSERT(handle->lckstream != NULL, return 0);
> > > > > -
> > > > > - fclose(handle->lckstream);
> > > > > - handle->lckstream = NULL;
> > > > > + ASSERT(handle->lckstream >= 0, return 0);
> > > > >
> > > > > - if(unlink(handle->lockfile) && errno !=
> > > > > ENOENT) {
> > > > > - return -1;
> > > > > + flock(handle->lckstream, LOCK_UN);
> > > > > + if(flock(handle->lckstream, LOCK_EX | LOCK_NB)
> > > > > == 0) {
> > > > > + unlink(handle->lockfile);
> > > > > + flock(handle->lckstream, LOCK_UN);
> > > > > }
> > > > > + handle->lckstream = NULL;
> > > > > return 0;
> > > > > }
> > > > >
> > > > > diff --git a/lib/libalpm/handle.h
> > > > > b/lib/libalpm/handle.h index 5e84d58..7ca90ff 100644
> > > > > --- a/lib/libalpm/handle.h
> > > > > +++ b/lib/libalpm/handle.h
> > > > > @@ -55,7 +55,7 @@ struct __alpm_handle_t {
> > > > > alpm_db_t *db_local; /* local db pointer
> > > > > */ alpm_list_t *dbs_sync; /* List of (alpm_db_t *)
> > > > > */ FILE *logstream; /* log file stream
> > > > > pointer */
> > > > > - FILE *lckstream; /* lock file stream
> > > > > pointer if one exists */
> > > > > + int lckstream; /* lock file file
> > > > > descriptor if one exists */ alpm_trans_t *trans;
> > > > >
> > > > > #ifdef HAVE_LIBCURL
> > > > > diff --git a/lib/libalpm/trans.c
> > > > > b/lib/libalpm/trans.c index 8d4e0e7..122f35b 100644
> > > > > --- a/lib/libalpm/trans.c
> > > > > +++ b/lib/libalpm/trans.c
> > > > > @@ -215,12 +215,7 @@ int SYMEXPORT
> > > > > alpm_trans_release(alpm_handle_t *handle)
> > > > > /* unlock db */
> > > > > if(!nolock_flag) {
> > > > > - if(_alpm_handle_unlock(handle)) {
> > > > > - _alpm_log(handle,
> > > > > ALPM_LOG_WARNING, _("could not remove lock file
> > > > > %s\n"), -
> > > > > handle->lockfile);
> > > > > - alpm_logaction(handle,
> > > > > ALPM_CALLER_PREFIX,
> > > > > - "warning: could not
> > > > > remove lock file %s\n", handle->lockfile);
> > > > > - }
> > > > > + _alpm_handle_unlock(handle);
> > > > > }
> > > > >
> > > > > return 0;
> > > > > diff --git a/scripts/pacman-db-upgrade.sh.in
> > > > > b/scripts/pacman-db-upgrade.sh.in index
> > > > > a1630c5..20cf035 100644 ---
> > > > > a/scripts/pacman-db-upgrade.sh.in +++
> > > > > b/scripts/pacman-db-upgrade.sh.in @@ -102,12
> > > > > +102,12 @@ dbroot="${dbroot%/}" # form the path to
> > > > > our lockfile location lockfile="${dbroot}/db.lck"
> > > > >
> > > > > -# make sure pacman isn't running
> > > > > -if [[ -f $lockfile ]]; then
> > > > > +# make sure pacman isn't running and stop other
> > > > > instances for starting +exec 100<>"$lockfile"
> > > > > +if ! flock -x --nonblock 100; then
> > > > > + flock --close 100
> > > > > die "$(gettext "Pacman lock file was found.
> > > > > Cannot run while pacman is running.")" fi
> > > > > -# do not let pacman run while we do this
> > > > > -touch "$lockfile"
> > > > >
> > > > > # pacman-3.4 to 3.5 upgrade - merge depends into
> > > > > desc if [[ $(find "$dbroot"/local -name depends)
> > > > > ]]; then @@ -123,5 +123,6 @@ fi
> > > > >
> > > > > # remove the lock file
> > > > > rm -f "$lockfile"
> > > > > +flock --close 100
> > > > >
> > > > > # vim: set ts=2 sw=2 noet:
> > > > > diff --git a/scripts/pacman-optimize.sh.in
> > > > > b/scripts/pacman-optimize.sh.in index
> > > > > 47fbb49..8c6e556 100644 ---
> > > > > a/scripts/pacman-optimize.sh.in +++
> > > > > b/scripts/pacman-optimize.sh.in @@ -114,12 +114,12
> > > > > @@ dbroot="${dbroot%/}" lockfile="${dbroot}/db.lck"
> > > > > localdb="${dbroot}/local"
> > > > >
> > > > > -# make sure pacman isn't running
> > > > > -if [[ -f $lockfile ]]; then
> > > > > +# make sure pacman isn't running and stop other
> > > > > instances for starting +exec 100<>"$lockfile"
> > > > > +if ! flock -x --nonblock 100; then
> > > > > + flock --close 100
> > > > > die "$(gettext "Pacman lock file was found.
> > > > > Cannot run while pacman is running.")" fi
> > > > > -# do not let pacman run while we do this
> > > > > -touch "$lockfile"
> > > > >
> > > > > workdir=$(mktemp -d
> > > > > "${TMPDIR:-/tmp}/pacman-optimize.XXXXXXXXXX") ||
> > > > > die_r "$(gettext "Can not create temp directory for
> > > > > database building.")\n" >&2 @@ -178,6 +178,7 @@ rm
> > > > > -rf "$localdb.old" # remove the lock file and our
> > > > > working directory with sums and tarfile rm -f
> > > > > "$lockfile" +flock --close 100
> > > > > rm -rf "$workdir"
> > > > >
> > > > > echo
> > > > > --
> > > > > 1.8.3.4
> > > > >
> > > > >
> > > >
> > >
> > >
> >
>
>
More information about the pacman-dev
mailing list