Patchwork [STABLE] transaction: abort transaction during hook exception

login
register
mail settings
Submitter Durham Goode
Date Jan. 19, 2016, 11:21 p.m.
Message ID <ccb47951e65f86e86676.1453245687@dev8486.prn1.facebook.com>
Download mbox | patch
Permalink /patch/12837/
State Accepted
Headers show

Comments

Durham Goode - Jan. 19, 2016, 11:21 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1453245501 28800
#      Tue Jan 19 15:18:21 2016 -0800
# Branch stable
# Node ID ccb47951e65f86e86676f07648e5591d601d3dd3
# Parent  d2c5ad3deccb5a504e2553652b66a4110db68afb
transaction: abort transaction during hook exception

The new transaction context did not handle the case where an exception during
close should still call release. This cause pretxnclose hooks that failed to
cause the transaction to fail without aborting, thus requiring a hg recover.

I've added a test.
Bryan O'Sullivan - Jan. 22, 2016, 1:21 a.m.
On Tue, Jan 19, 2016 at 3:21 PM, Durham Goode <durham@fb.com> wrote:

> transaction: abort transaction during hook exception
>

Nicely caught.
Bryan O'Sullivan - Jan. 22, 2016, 3:39 a.m.
On Thu, Jan 21, 2016 at 5:21 PM, Bryan O'Sullivan <bos@serpentine.com>
wrote:

>
> On Tue, Jan 19, 2016 at 3:21 PM, Durham Goode <durham@fb.com> wrote:
>
>> transaction: abort transaction during hook exception
>>
>
> Nicely caught.
>

...aaaand pushed to the clowncopter.

Patch

diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -337,9 +337,11 @@  class transaction(object):
         return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):
-        if exc_type is None:
-            self.close()
-        self.release()
+        try:
+            if exc_type is None:
+                self.close()
+        finally:
+            self.release()
 
     def running(self):
         return self.count > 0
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -724,3 +724,25 @@  new commits must be visible in pretxncha
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     b
   
+  $ cd ..
+
+pretxnclose hook failure should abort the transaction
+
+  $ hg init txnfailure
+  $ cd txnfailure
+  $ touch a && hg commit -Aqm a
+  $ cat >> .hg/hgrc <<EOF
+  > [hooks]
+  > pretxnclose.error = exit 1
+  > EOF
+  $ hg strip -r 0 --config extensions.strip=
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  saved backup bundle to * (glob)
+  transaction abort!
+  rollback completed
+  strip failed, full bundle stored in * (glob)
+  abort: pretxnclose.error hook exited with status 1
+  [255]
+  $ hg recover
+  no interrupted transaction available
+  [1]