Patchwork test-bundle2-format.t: force gc so a GeneratorExit will be thrown

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 2, 2016, 8:40 a.m.
Message ID <20160102174003.7ea8b1610ebbb8a74fcc21f1@tcha.org>
Download mbox | patch
Permalink /patch/12470/
State Not Applicable
Headers show

Comments

Yuya Nishihara - Jan. 2, 2016, 8:40 a.m.
On Fri, 1 Jan 2016 10:51:55 -0800, Bryan O'Sullivan wrote:
> On Tue, Dec 29, 2015 at 2:55 PM, Augie Fackler <raf@durin42.com> wrote:
> > I'm a little weirded out by this fix, because it worries me that it is
> > going to choke us in weird ways in production. Can you allay that fear at
> > all?
> 
> I don't think it's really the right fix myself – we need something more
> deterministic than relying on the refcount dropping to zero or the gc
> running.

But it shouldn't be deterministic when a generator is consumed unless next()
is called. If we want to make this test passed without gc.collect(), all "for"
loops have to call generator.close().

So I'm inclined to gc.collect() is fine here to forcibly raise and process
GeneratorExit.

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -549,8 +549,12 @@  class bundle20(object):
         if param:
             yield param
         # starting compression
-        for chunk in self._getcorechunk():
-            yield self._compressor.compress(chunk)
+        g = self._getcorechunk()
+        try:
+            for chunk in g:
+                yield self._compressor.compress(chunk)
+        finally:
+            g.close()
         yield self._compressor.flush()
 
     def _paramchunk(self):
@@ -571,8 +575,12 @@  class bundle20(object):
         outdebug(self.ui, 'start of parts')
         for part in self._parts:
             outdebug(self.ui, 'bundle part: "%s"' % part.type)
-            for chunk in part.getchunks(ui=self.ui):
-                yield chunk
+            g = part.getchunks(ui=self.ui)
+            try:
+                for chunk in g:
+                    yield chunk
+            finally:
+                g.close()
         outdebug(self.ui, 'end of bundle')
         yield _pack(_fpartheadersize, 0)
 
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
@@ -150,7 +150,8 @@  Create an extension to test bundle2 API
   > 
   >     if opts['timeout']:
   >         bundler.newpart('test:song', data=ELEPHANTSSONG, mandatory=False)
-  >         for idx, junk in enumerate(bundler.getchunks()):
+  >         g = bundler.getchunks()
+  >         for idx, junk in enumerate(g):
   >             ui.write('%d chunk\n' % idx)
   >             if idx > 4:
   >                 # This throws a GeneratorExit inside the generator, which
@@ -158,7 +159,7 @@  Create an extension to test bundle2 API
   >                 # too zealous. It's important for this test that the break
   >                 # occur while we're in the middle of a part.
   >                 break
-  >         gc.collect()
+  >         g.close()
   >         ui.write('fake timeout complete.\n')
   >         return
   >     try: