Bug report: memory leak in python 1.5.2

[Fred L. Drake]

M.-A. Lemburg writes:
 > Don't why the free(new); is missing though... maybe some putenv()
 > implementation expect a malloced string (the putenv implementation
 > in posixmodule.c for NeXT does) while other make a copy first.

  At least on Solaris, the malloc()'ed memory must survive until the
same key is re-used.
  I think using a Python string for the malloc()'ed memory is
acceptable (it's just a buffer, after all).  I took a stab at using a
dictionary to "cache" the last string passed to putenv() for each key; 
when using the same key again, the old value is removed from the
dictionary and deleted.  Greg's code doesn't appear to leak with my
patch, at any rate (running for over 7 minutes on a 200Mhz
UltraSPARC).
  I've appended the patch below for others to try out and verify.  If
others get positive results and can't think of reasons not to accept
this, perhaps I can knock Guido over the head with it when he gets
back to town.  ;-)


  -Fred

--
Fred L. Drake, Jr.	     <fdrake at acm.org>
Corporation for National Research Initiatives

Index: posixmodule.c
===================================================================
RCS file: /projects/cvsroot/python/dist/src/Modules/posixmodule.c,v
retrieving revision 2.110
diff -c -c -r2.110 posixmodule.c
*** posixmodule.c	1999/04/07 15:49:41	2.110
--- posixmodule.c	1999/05/26 14:29:08
***************
*** 2776,2781 ****
--- 2776,2785 ----
  int putenv( const char *str );
  #endif
  
+ /* Save putenv() parameters as values here, so we can collect them when they
+  * get re-set with another call for the same key. */
+ static PyObject *posix_putenv_garbage;
+ 
  static PyObject * 
  posix_putenv(self, args)
  	PyObject *self;
***************
*** 2783,2788 ****
--- 2787,2793 ----
  {
          char *s1, *s2;
          char *new;
+ 	PyObject *newstr;
  
  	if (!PyArg_ParseTuple(args, "ss", &s1, &s2))
  		return NULL;
***************
*** 2810,2823 ****
      } else {
  #endif
  
! 	/* XXX This leaks memory -- not easy to fix :-( */
! 	if ((new = malloc(strlen(s1) + strlen(s2) + 2)) == NULL)
!                 return PyErr_NoMemory();
  	(void) sprintf(new, "%s=%s", s1, s2);
  	if (putenv(new)) {
                  posix_error();
                  return NULL;
  	}
  
  #if defined(PYOS_OS2)
      }
--- 2815,2842 ----
      } else {
  #endif
  
! 	/* XXX This can leak memory -- not easy to fix :-( */
! 	newstr = PyString_FromStringAndSize(NULL, strlen(s1) + strlen(s2) + 2);
! 	if (newstr == NULL)
! 		return PyErr_NoMemory();
! 	new = PyString_AS_STRING(newstr);
  	(void) sprintf(new, "%s=%s", s1, s2);
  	if (putenv(new)) {
                  posix_error();
                  return NULL;
  	}
+ 	/* Install the first arg and newstr in posix_putenv_garbage;
+ 	 * this will cause previous value to be collected.  This has to
+ 	 * happen after the real putenv() call because the old value
+ 	 * was still accessible until then. */
+ 	if (PyDict_SetItem(posix_putenv_garbage,
+ 			   PyTuple_GET_ITEM(args, 0), newstr)) {
+ 		/* really not much we can do; just leak */
+ 		PyErr_Clear();
+ 	}
+ 	else {
+ 		Py_DECREF(newstr);
+ 	}
  
  #if defined(PYOS_OS2)
      }
***************
*** 3489,3492 ****
--- 3508,3513 ----
                  return;
  
  	PyDict_SetItemString(d, "error", PyExc_OSError);
+ 
+ 	posix_putenv_garbage = PyDict_New();
  }