Patchwork lazymanifest: fix memory leak in lmiter_iterentriesnext() after 3d485727e45e

login
register
mail settings
Submitter Mike Hommey
Date April 12, 2015, 8:42 a.m.
Message ID <1428828157-5536-1-git-send-email-mh@glandium.org>
Download mbox | patch
Permalink /patch/8621/
State Accepted
Headers show

Comments

Mike Hommey - April 12, 2015, 8:42 a.m.

Mike Hommey - April 12, 2015, 8:45 a.m.
On Sun, Apr 12, 2015 at 05:42:37PM +0900, Mike Hommey wrote:
> 
> diff --git a/mercurial/manifest.c b/mercurial/manifest.c
> index 2b12d55..8688c49 100644
> --- a/mercurial/manifest.c
> +++ b/mercurial/manifest.c
> @@ -232,7 +232,7 @@ static PyObject *lmiter_iterentriesnext(PyObject *o)
>  	size_t pl;
>  	line *l;
>  	Py_ssize_t consumed;
> -	PyObject *path = NULL, *hash = NULL, *flags = NULL;
> +	PyObject *ret = NULL, *path = NULL, *hash = NULL, *flags = NULL;
>  	l = lmiter_nextline((lmIter *)o);
>  	if (!l) {
>  		goto bail;
> @@ -246,12 +246,12 @@ static PyObject *lmiter_iterentriesnext(PyObject *o)
>  	if (!path || !hash || !flags) {
>  		goto bail;
>  	}
> -	return PyTuple_Pack(3, path, hash, flags);
> +	ret = PyTuple_Pack(3, path, hash, flags);
>   bail:
>  	Py_XDECREF(path);
>  	Py_XDECREF(hash);
>  	Py_XDECREF(flags);
> -	return NULL;
> +	return ret;
>  }
>  
>  static PyTypeObject lazymanifestEntriesIterator = {

One way to reliably reproduce this:
$ hg clone -U http://hg.mozilla.org/mozilla-central
$ cd mozilla-central
$ hg bundle  --base 0 -r c64d97a1ebd769831a69410b1064749bf7630a64 foo.hg

Mike
Martin von Zweigbergk - April 12, 2015, 1:55 p.m.
Ouch, that's a pretty bad leak. Sorry :-( I've pushed this to the
clowncopter. Thanks!

So the "bail" code had to run even in the non-bail case. Perhaps I'll send
a patch to rename "bail" to "cleanup" on top of this patch, so this is less
likely to happen again.

On Sun, Apr 12, 2015 at 1:46 AM Mike Hommey <mh@glandium.org> wrote:

> On Sun, Apr 12, 2015 at 05:42:37PM +0900, Mike Hommey wrote:
> >
> > diff --git a/mercurial/manifest.c b/mercurial/manifest.c
> > index 2b12d55..8688c49 100644
> > --- a/mercurial/manifest.c
> > +++ b/mercurial/manifest.c
> > @@ -232,7 +232,7 @@ static PyObject *lmiter_iterentriesnext(PyObject *o)
> >       size_t pl;
> >       line *l;
> >       Py_ssize_t consumed;
> > -     PyObject *path = NULL, *hash = NULL, *flags = NULL;
> > +     PyObject *ret = NULL, *path = NULL, *hash = NULL, *flags = NULL;
> >       l = lmiter_nextline((lmIter *)o);
> >       if (!l) {
> >               goto bail;
> > @@ -246,12 +246,12 @@ static PyObject *lmiter_iterentriesnext(PyObject
> *o)
> >       if (!path || !hash || !flags) {
> >               goto bail;
> >       }
> > -     return PyTuple_Pack(3, path, hash, flags);
> > +     ret = PyTuple_Pack(3, path, hash, flags);
> >   bail:
> >       Py_XDECREF(path);
> >       Py_XDECREF(hash);
> >       Py_XDECREF(flags);
> > -     return NULL;
> > +     return ret;
> >  }
> >
> >  static PyTypeObject lazymanifestEntriesIterator = {
>
> One way to reliably reproduce this:
> $ hg clone -U http://hg.mozilla.org/mozilla-central
> $ cd mozilla-central
> $ hg bundle  --base 0 -r c64d97a1ebd769831a69410b1064749bf7630a64 foo.hg
>
> Mike
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/manifest.c b/mercurial/manifest.c
index 2b12d55..8688c49 100644
--- a/mercurial/manifest.c
+++ b/mercurial/manifest.c
@@ -232,7 +232,7 @@  static PyObject *lmiter_iterentriesnext(PyObject *o)
 	size_t pl;
 	line *l;
 	Py_ssize_t consumed;
-	PyObject *path = NULL, *hash = NULL, *flags = NULL;
+	PyObject *ret = NULL, *path = NULL, *hash = NULL, *flags = NULL;
 	l = lmiter_nextline((lmIter *)o);
 	if (!l) {
 		goto bail;
@@ -246,12 +246,12 @@  static PyObject *lmiter_iterentriesnext(PyObject *o)
 	if (!path || !hash || !flags) {
 		goto bail;
 	}
-	return PyTuple_Pack(3, path, hash, flags);
+	ret = PyTuple_Pack(3, path, hash, flags);
  bail:
 	Py_XDECREF(path);
 	Py_XDECREF(hash);
 	Py_XDECREF(flags);
-	return NULL;
+	return ret;
 }
 
 static PyTypeObject lazymanifestEntriesIterator = {