Patchwork bundle2: don't try to recover from a GeneratorExit (issue4785)

login
register
mail settings
Submitter Augie Fackler
Date Sept. 1, 2015, 7:50 p.m.
Message ID <b6df80437efc550628f7.1441137021@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/10356/
State Accepted
Headers show

Comments

Augie Fackler - Sept. 1, 2015, 7:50 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1441136853 14400
#      Tue Sep 01 15:47:33 2015 -0400
# Node ID b6df80437efc550628f7160a9730779f4480b8b3
# Parent  30104aecdcadc579f5532142620ef366e218eefc
bundle2: don't try to recover from a GeneratorExit (issue4785)

GeneratorExit means the other end of the conversation has already
stopped listening, so don't try and yield out error
information. Instead, just let the GeneratorExit propagate normally.

This should resolve esoteric issues observed with servers that have
aggressive timeouts waiting for data to send to clients logging
internal Python errors[0]. This has been observed with both gunicorn's
gevent worker model and with scm-manager's built-in webserver (which
itself is something sitting inside jetty.)


0: Exception RuntimeError: 'generator ignored GeneratorExit' in <generator object getchunks at 0x7fd2f6c586e0> ignored
Matt Mackall - Sept. 2, 2015, 10:33 p.m.
On Tue, 2015-09-01 at 15:50 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1441136853 14400
> #      Tue Sep 01 15:47:33 2015 -0400
> # Node ID b6df80437efc550628f7160a9730779f4480b8b3
> # Parent  30104aecdcadc579f5532142620ef366e218eefc
> bundle2: don't try to recover from a GeneratorExit (issue4785)

Queued for default, thanks.

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -847,6 +847,12 @@  class bundlepart(object):
                 outdebug(ui, 'payload chunk size: %i' % len(chunk))
                 yield _pack(_fpayloadsize, len(chunk))
                 yield chunk
+        except GeneratorExit:
+            # GeneratorExit means that nobody is listening for our
+            # results anyway, so just bail quickly rather than trying
+            # to produce an error part.
+            ui.debug('bundle2-generatorexit\n')
+            raise
         except BaseException as exc:
             # backup exception data for later
             ui.debug('bundle2-input-stream-interrupt: encoding exception %s'
diff --git a/tests/test-bundle2-format.t b/tests/test-bundle2-format.t
--- a/tests/test-bundle2-format.t
+++ b/tests/test-bundle2-format.t
@@ -78,6 +78,7 @@  Create an extension to test bundle2 API
   >           ('', 'reply', False, 'produce a reply bundle'),
   >           ('', 'pushrace', False, 'includes a check:head part with unknown nodes'),
   >           ('', 'genraise', False, 'includes a part that raise an exception during generation'),
+  >           ('', 'timeout', False, 'emulate a timeout during bundle generation'),
   >           ('r', 'rev', [], 'includes those changeset in the bundle'),],
   >          '[OUTPUTFILE]')
   > def cmdbundle2(ui, repo, path=None, **opts):
@@ -143,6 +144,18 @@  Create an extension to test bundle2 API
   >     else:
   >         file = open(path, 'wb')
   > 
+  >     if opts['timeout']:
+  >         bundler.newpart('test:song', data=ELEPHANTSSONG, mandatory=False)
+  >         for idx, junk in enumerate(bundler.getchunks()):
+  >             ui.write('%d chunk\n' % idx)
+  >             if idx > 4:
+  >                 # This throws a GeneratorExit inside the generator, which
+  >                 # can cause problems if the exception-recovery code is
+  >                 # too zealous. It's important for this test that the break
+  >                 # occur while we're in the middle of a part.
+  >                 break
+  >         ui.write('fake timeout complete.\n')
+  >         return
   >     try:
   >         for chunk in bundler.getchunks():
   >             file.write(chunk)
@@ -239,6 +252,26 @@  Test bundling
   $ hg bundle2
   HG20\x00\x00\x00\x00\x00\x00\x00\x00 (no-eol) (esc)
 
+Test timeouts during bundling
+  $ hg bundle2 --timeout --debug --config devel.bundle2.debug=yes
+  bundle2-output-bundle: "HG20", 1 parts total
+  bundle2-output: start emission of HG20 stream
+  0 chunk
+  bundle2-output: bundle parameter: 
+  1 chunk
+  bundle2-output: start of parts
+  bundle2-output: bundle part: "test:song"
+  bundle2-output-part: "test:song" (advisory) 178 bytes payload
+  bundle2-output: part 0: "test:song"
+  bundle2-output: header chunk size: 16
+  2 chunk
+  3 chunk
+  bundle2-output: payload chunk size: 178
+  4 chunk
+  5 chunk
+  bundle2-generatorexit
+  fake timeout complete.
+
 Test unbundling
 
   $ hg bundle2 | hg statbundle2