Patchwork [5,of,5] lock.release: do not unlink inherited locks

login
register
mail settings
Submitter Siddharth Agarwal
Date Sept. 24, 2015, 11:29 p.m.
Message ID <6d6573be7dc37376d9c5.1443137380@dev6666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/10623/
State Accepted
Headers show

Comments

Siddharth Agarwal - Sept. 24, 2015, 11:29 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1443135806 25200
#      Thu Sep 24 16:03:26 2015 -0700
# Node ID 6d6573be7dc37376d9c5a22be8fbbfcdc4f2814a
# Parent  ba556998f3d950a16bd890a1d7f23ba8cdb6801c
lock.release: do not unlink inherited locks

This is part of a series that will allow locks to be inherited by subprocesses
in limited circumstances. A subprocess unlinking a lock will lead to potential
corruption from other concurrent processes.
Augie Fackler - Sept. 25, 2015, 5:37 p.m.
On Thu, Sep 24, 2015 at 04:29:40PM -0700, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1443135806 25200
> #      Thu Sep 24 16:03:26 2015 -0700
> # Node ID 6d6573be7dc37376d9c5a22be8fbbfcdc4f2814a
> # Parent  ba556998f3d950a16bd890a1d7f23ba8cdb6801c
> lock.release: do not unlink inherited locks

Queued, thanks. What's the use case?

>
> This is part of a series that will allow locks to be inherited by subprocesses
> in limited circumstances. A subprocess unlinking a lock will lead to potential
> corruption from other concurrent processes.
>
> diff --git a/mercurial/lock.py b/mercurial/lock.py
> --- a/mercurial/lock.py
> +++ b/mercurial/lock.py
> @@ -204,10 +204,11 @@ class lock(object):
>                  if self.releasefn:
>                      self.releasefn()
>              finally:
> -                try:
> -                    self.vfs.unlink(self.f)
> -                except OSError:
> -                    pass
> +                if not self._parentheld:
> +                    try:
> +                        self.vfs.unlink(self.f)
> +                    except OSError:
> +                        pass
>              for callback in self.postrelease:
>                  callback()
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Siddharth Agarwal - Sept. 25, 2015, 6:04 p.m.
On 9/25/15 10:37 AM, Augie Fackler wrote:
> On Thu, Sep 24, 2015 at 04:29:40PM -0700, Siddharth Agarwal wrote:
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1443135806 25200
>> #      Thu Sep 24 16:03:26 2015 -0700
>> # Node ID 6d6573be7dc37376d9c5a22be8fbbfcdc4f2814a
>> # Parent  ba556998f3d950a16bd890a1d7f23ba8cdb6801c
>> lock.release: do not unlink inherited locks
> Queued, thanks. What's the use case?

The overall goal is to allow a custom driver for the overall merge 
process, which can:
(1) use the right tools for merges, with more power than the glob 
patterns that merge-patterns lets us specify at the moment. For example, 
multiple files resolved by a single tool, or generated files. The 
merge-patterns stuff is really geared towards tools like kdiff3/meld -- 
this is more general than that.
(2) provide an ordering to merges. For example, generated files should 
only be resolved after source files.

This was briefly discussed at the last Mercurial sprint in Montreal, and 
we now have a burning need for this.

I have a WIP series at http://42.netv6.net/sid0-wip/hg/graph/autoresolve 
-- it's not fully cleaned up though.

>
>> This is part of a series that will allow locks to be inherited by subprocesses
>> in limited circumstances. A subprocess unlinking a lock will lead to potential
>> corruption from other concurrent processes.
>>
>> diff --git a/mercurial/lock.py b/mercurial/lock.py
>> --- a/mercurial/lock.py
>> +++ b/mercurial/lock.py
>> @@ -204,10 +204,11 @@ class lock(object):
>>                   if self.releasefn:
>>                       self.releasefn()
>>               finally:
>> -                try:
>> -                    self.vfs.unlink(self.f)
>> -                except OSError:
>> -                    pass
>> +                if not self._parentheld:
>> +                    try:
>> +                        self.vfs.unlink(self.f)
>> +                    except OSError:
>> +                        pass
>>               for callback in self.postrelease:
>>                   callback()
>>
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> https://selenic.com/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -204,10 +204,11 @@  class lock(object):
                 if self.releasefn:
                     self.releasefn()
             finally:
-                try:
-                    self.vfs.unlink(self.f)
-                except OSError:
-                    pass
+                if not self._parentheld:
+                    try:
+                        self.vfs.unlink(self.f)
+                    except OSError:
+                        pass
             for callback in self.postrelease:
                 callback()