[PATCH 2 of 3] chg: start server at a unique address

Gregory Szorc gregory.szorc at gmail.com
Sat Dec 17 19:40:12 EST 2016


It's probably not worth worrying about just yet, but with Linux PID namespaces, multiple processes may think they have the same PID, even if that PID maps to different values inside the kernel.

Mozilla has encountered problems with multiple hg processes running in separate containers and PID namespaces acquiring the same lock from a shared filesystem simultaneously because multiple hg processes share the same PID and hostname in different "containers."

> On Dec 16, 2016, at 17:49, Jun Wu <quark at fb.com> wrote:
> 
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1481938472 0
> #      Sat Dec 17 01:34:32 2016 +0000
> # Node ID 6c9ce8399350d8287599cd802b91adf73db08759
> # Parent  69d25b06467d65bf6d1e85d34d8fc57ec321b51d
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 6c9ce8399350
> chg: start server at a unique address
> 
> See the previous patch for motivation. Previously, the server is started at
> a globally shared address, which requires a lock. This patch appends pid to
> the address so it becomes unique.
> 
> diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
> --- a/contrib/chg/chg.c
> +++ b/contrib/chg/chg.c
> @@ -32,4 +32,5 @@
> struct cmdserveropts {
>    char sockname[UNIX_PATH_MAX];
> +    char initsockname[UNIX_PATH_MAX];
>    char redirectsockname[UNIX_PATH_MAX];
>    char lockfile[UNIX_PATH_MAX];
> @@ -164,4 +165,8 @@ static void setcmdserveropts(struct cmds
>    if (r < 0 || (size_t)r >= sizeof(opts->lockfile))
>        abortmsg("too long TMPDIR or CHGSOCKNAME (r = %d)", r);
> +    r = snprintf(opts->initsockname, sizeof(opts->initsockname),
> +            "%s.%u", opts->sockname, (unsigned)getpid());
> +    if (r < 0 || (size_t)r >= sizeof(opts->initsockname))
> +        abortmsg("too long TMPDIR or CHGSOCKNAME (r = %d)", r);
> }
> 
> @@ -224,5 +229,5 @@ static void execcmdserver(const struct c
>        "serve",
>        "--cmdserver", "chgunix",
> -        "--address", opts->sockname,
> +        "--address", opts->initsockname,
>        "--daemon-postexec", "chdir:/",
>    };
> @@ -248,5 +253,5 @@ static hgclient_t *retryconnectcmdserver
>    int pst = 0;
> 
> -    debugmsg("try connect to %s repeatedly", opts->sockname);
> +    debugmsg("try connect to %s repeatedly", opts->initsockname);
> 
>    unsigned int timeoutsec = 60;  /* default: 60 seconds */
> @@ -256,7 +261,13 @@ static hgclient_t *retryconnectcmdserver
> 
>    for (unsigned int i = 0; !timeoutsec || i < timeoutsec * 100; i++) {
> -        hgclient_t *hgc = hgc_open(opts->sockname);
> -        if (hgc)
> +        hgclient_t *hgc = hgc_open(opts->initsockname);
> +        if (hgc) {
> +            debugmsg("rename %s to %s", opts->initsockname,
> +                    opts->sockname);
> +            int r = rename(opts->initsockname, opts->sockname);
> +            if (r != 0)
> +                abortmsgerrno("cannot rename");
>            return hgc;
> +        }
> 
>        if (pid > 0) {
> @@ -270,5 +281,5 @@ static hgclient_t *retryconnectcmdserver
>    }
> 
> -    abortmsg("timed out waiting for cmdserver %s", opts->sockname);
> +    abortmsg("timed out waiting for cmdserver %s", opts->initsockname);
>    return NULL;
> 
> @@ -313,5 +324,5 @@ static hgclient_t *connectcmdserver(stru
>        unlink(opts->sockname);
> 
> -    debugmsg("start cmdserver at %s", opts->sockname);
> +    debugmsg("start cmdserver at %s", opts->initsockname);
> 
>    pid_t pid = fork();
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list