[PATCH 2 of 2] lock: include Linux pid namespace identifier in prefix

[Augie Fackler]

On Fri, Feb 10, 2017 at 04:50:55PM -0800, Jun Wu wrote:
> I'd like to note that although this patch prevents repo corruption when
> running hg inside different containers (which has different pid namespaces),
> it does not prevent deadlock - if an hg process is SIGKILL-ed, every other
> process will not able to take or remove the lock.

Sigh. Thanks for the fix, queued (and a fist shaken at this weird/poor
choice from linux containers.)

>
> I think if we do know the repo is not on NFS, and the system supports
> flock(), flock() is way more robust and solves all kinds of pain here.
>
> I hereby propose a new repo requirement "flock", once set, use flock instead
> of the traditional lock file. It's off by default. Thoughts?

I'm...not categorically opposed to it, though it feels pretty risky. I
know git doesn't use flock() either - presumably there's a good reason
I don't know about that neither tool relies on it?

>
> Excerpts from Jun Wu's message of 2017-02-10 14:09:35 -0800:
> > # HG changeset patch
> > # User Jun Wu <quark at fb.com>
> > # Date 1486763791 28800
> > #      Fri Feb 10 13:56:31 2017 -0800
> > # Node ID 1884652829f6e68bde6ab3e9b0e9ca1da9bed40c
> > # Parent  f8eb762559b242ec12b199db1ce27c2930881c75
> > # Available At https://bitbucket.org/quark-zju/hg-draft
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 1884652829f6
> > lock: include Linux pid namespace identifier in prefix
> >
> > Previously, the lock only contains a hostname as an attempt to detect pid
> > namespace difference. However, that's not enough on modern Linux - a single
> > hostname could have different pid namespaces.
> >
> > That means if people run hg inside different PID namespaces with a same UTS
> > namespae, the lock would be broken - an hg proccess in pid namespace A will
> > think the lock having a "random" pid in pid namespace B is "dead" and remove
> > it.
> >
> > This patch solves the above issue by appending an PID namespace identifier of
> > the current process to the lock prefix ("hostname"). It depends on /proc
> > being mounted properly. But I don't think there is a better way to get pid
> > namespace identifier reliably.
> >
> > diff --git a/mercurial/lock.py b/mercurial/lock.py
> > --- a/mercurial/lock.py
> > +++ b/mercurial/lock.py
> > @@ -10,4 +10,5 @@ from __future__ import absolute_import
> >  import contextlib
> >  import errno
> > +import os
> >  import socket
> >  import time
> > @@ -16,4 +17,5 @@ import warnings
> >  from . import (
> >      error,
> > +    pycompat,
> >      util,
> >  )
> > @@ -23,7 +25,15 @@ def _getlockprefix():
> >
> >      It's useful to detect "dead" processes and remove stale locks with
> > -    confidence. Typically it's just hostname.
> > +    confidence. Typically it's just hostname. On modern linux, we include an
> > +    extra Linux-specific pid namespace identifier.
> >      """
> > -    return socket.gethostname()
> > +    result = socket.gethostname()
> > +    if pycompat.sysplatform.startswith('linux'):
> > +        try:
> > +            result += '/%x' % os.stat('/proc/self/ns/pid').st_ino
> > +        except OSError as ex:
> > +            if ex.errno not in (errno.ENOENT, errno.EACCES, errno.ENOTDIR):
> > +                raise
> > +    return result
> >
> >  class lock(object):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel