Tcl Source Code

When PUTENV is set on windows 2000, the comment 
appears to be right, but the code doesn't follow 
through.
    /*
     * Watch out for versions of putenv that copy the 
string (e.g. VC++).
     * In this case we need to free the string 
immediately.  Otherwise
     * update the string in the cache.
     */
(environ[index] != p) the buffer is never freed.

Logged In: YES 
user_id=7549

Patch committed to HEAD.

No negative response heard from my eyeball request on the 
core team mailing list, so therefore this gets pushed.

File Deleted - 29691: 



File Added - 29722: patch.txt

Logged In: YES 
user_id=7549

Jeff, could you have a look at this patch?  It looks ready.

With Dave Welton on the assist, here we go for more.  Now 
tested on NetBSD 1.5.2 (which does copy), and windows 
(makefile.vc) by myself.  It looks good.

File Deleted - 29669: 



File Added - 29691: patch.txt

Logged In: YES 
user_id=7549

new patch attached with unix/configure.in test added for 
putenv() copying.  Is this ready to be applied?

Logged In: YES 
user_id=240

#include <stdlib.h>

#define OURVAR "hola=chau"

int main (int argc, char *argv[], char *environ)
{
    char *foo;
    char *bar;
    foo = (char *)strdup(OURVAR);

    putenv(foo);
    bar = getenv("hola");
    printf("%x == %x\n", bar, strchr(foo, '=') + 1);

}

Is one example of how we could test for a copying putenv.

File Deleted - 23833: 



File Added - 29669: patch.txt

Logged In: YES 
user_id=7549

newer patch added.  Assuming that VC++ 7.0 has that 
memory leak fixed, we still have a putenv() that copies.  A 
good assumption, at least.  According to David Welton in 
private email glibc 2.0 - 2.1.1 also has a copy behavior and 
BSD4.4 does as well.  I think it would be wise to add a 
configure test based on behavior rather than glibc versions.

Could someone do this for us?

Logged In: YES 
user_id=240

I am seeing this on windows XP as well, just FYI.

Logged In: YES 
user_id=7549

All this caching makes me sick.  Please rekindle my faith in 
putenv() and what is wrong with the Win32's 
Put/GetEnvironmentVariable(), GetEnvironmentStrings()?

GetEnvironmentVariable() will need 2 calls, if the static buffer 
isn't large enough.  Is this really a speed issue with this 
caching stuff?

Logged In: YES 
user_id=7549

If this leak is truly caused by the MS runtime, why is
ckrealloc() the originator of the memory blocks as shown
in the trace dumps?

 1e5d1a0 -  1e5d1af       16 @ ..\generic\tclEnv.c 249 
envLeak18
 1e4a4b8 -  1e4a4c5       14 @ ..\generic\tclEnv.c 249 
envLeak18
 1e4a470 -  1e4a47f       16 @ ..\generic\tclEnv.c 249 
envLeak18
 1e4a428 -  1e4a435       14 @ ..\generic\tclEnv.c 249 
envLeak18
 1f99758 -  1f99762       11 @ ..\generic\tclEnv.c 249 
envLeak18
 1e4a688 -  1e4a692       11 @ ..\generic\tclEnv.c 249 
envLeak18
  981930 -   98193d       14 @ ..\generic\tclEnv.c 249 
envLeak18
 1d8c348 -  1d8c35b       20 @ ..\generic\tclEnv.c 249 
envLeak18
  8f05e8 -   8f05fe       23 @ ..\generic\tclEnv.c 249 envLeak18
 1f98b38 -  1f98b42       11 @ ..\generic\tclEnv.c 249 
envLeak18
 1e4a398 -  1e4a3a4       13 @ ..\generic\tclEnv.c 249 
envLeak18
 1e4a3e0 -  1e4a3ec       13 @ ..\generic\tclEnv.c 249 
envLeak18
 1fbfff0 -  1fbfff3        4 @ ..\generic\tclEnv.c 409 
 1fb9070 -  1fb9076        7 @ ..\generic\tclEnv.c 249 
 1e5d868 -  1e5d877       16 @ ..\generic\tclEnv.c 249 
envLeak17
 1e4a100 -  1e4a10d       14 @ ..\generic\tclEnv.c 249 
envLeak17
 1e4a0b8 -  1e4a0c7       16 @ ..\generic\tclEnv.c 249 
envLeak17
 1e4a070 -  1e4a07d       14 @ ..\generic\tclEnv.c 249 
envLeak17
 1f98ca0 -  1f98caa       11 @ ..\generic\tclEnv.c 249 
envLeak17
 1fbf598 -  1fbf5a2       11 @ ..\generic\tclEnv.c 249 envLeak17
  957d08 -   957d15       14 @ ..\generic\tclEnv.c 249 
envLeak17
 1fb90b8 -  1fb90cb       20 @ ..\generic\tclEnv.c 249 
envLeak17
 1fa3378 -  1fa338e       23 @ ..\generic\tclEnv.c 249 
envLeak17
 1f985c8 -  1f985d2       11 @ ..\generic\tclEnv.c 249 
envLeak17
 1e4a7a8 -  1e4a7b4       13 @ ..\generic\tclEnv.c 249 
envLeak17
 1e4a028 -  1e4a034       13 @ ..\generic\tclEnv.c 249 
envLeak17

etc....

Logged In: YES 
user_id=7549

Just because VC++'s putenv() copies the string and might 
impart it's own memory leak in doing so, might be a seperate 
issue from Tcl not free'ing it's own buffer used for the putenv() 
call.

Logged In: YES 
user_id=7549

Apply this to stock tclhttpd, and slam it with a few hundred 
hits, then close the server down and read through the onexit 
dump.

Index: doc.tcl
=============================================
======================
RCS file: /cvsroot/tclhttpd/tclhttpd/lib/doc.tcl,v
retrieving revision 1.43
diff -c -r1.43 doc.tcl
*** doc.tcl9 Jul 2001 23:05:19 -00001.43
--- doc.tcl27 May 2002 21:51:00 -0000
***************
*** 858,864 ****
      upvar #0 Httpd$sock data
      set Doc(errorUrl) $data(url)
      set Doc(errorInfo) $ei;# For subst
!     CountName $Doc(errorUrl) error
      DocSubstSystemFile $sock error 500 [protect_text $ei]
  }
  
--- 858,864 ----
      upvar #0 Httpd$sock data
      set Doc(errorUrl) $data(url)
      set Doc(errorInfo) $ei;# For subst
!     CountName $Doc(errorUrl) errors
      DocSubstSystemFile $sock error 500 [protect_text $ei]
  }
  
***************
*** 1118,1125 ****
--- 1118,1139 ----
      # Populate the global "env" array similarly to the CGI 
environment
  
      Cgi_SetEnv $sock $filename pass
+ 
+ ### DAVYGRVY LEAK TESTING
+     global davygrvy
+     if {![info exist davygrvy]} {
+ set davygrvy -1
+ memory onexit onexit.txt
+     }
+     memory tag "envLeak[incr davygrvy]"
+ ### DAVYGRVY LEAK TESTING
+ 
      interp eval $interp [list uplevel #0 \
  [list array set env [array get pass]]]
+ 
+ ### DAVYGRVY LEAK TESTING
+     memory tag ""
+ ### DAVYGRVY LEAK TESTING
  
      if {0} {
  # Duplicate this in the "cgienv" array.

Logged In: YES 
user_id=7549

_MSC_VER == 1200 for VC6

A check against the actual runtime loaded (msvcrt.dll) might 
be more accurate.  I really don't know how VC++ .NET has 
this fixed.  Is it a new msvcrt.dll or what?  I'm assuming 
MingW uses a broken msvcrt.dll.

Logged In: YES 
user_id=72656

what sort of testing did you do this on?  Is the MSVC version 
check right so that it won't trigger in VS.NET, which 
supposedly has corrected this?

File Added - 23833: patch.txt

Logged In: YES 
user_id=7549

here's me trying to free it with success.  This does work.

This was the source of a 373 byte leak I was searching for in 
tclhttpd per page hit.  The leak is now down to 27 bytes with 
the patch applied.

Logged In: YES 
user_id=7549

For an alternate approach, one could consider not using 
_environ at all and expell effort maintaining a cache.  One 
could ask the OS directly with GetEnvironmentString() and 
parse it directly for the entries asked of it.

http://msdn.microsoft.com/library/en-
us/dllproc/prothred_9gs3.asp

We'll need a new platform specific source file.  It might 
impart a performance hit.  The first few entries in the 
string are a bit odd.  They probably have internal meaning, 
but aren't documented.

Logged In: YES 
user_id=72656

As indicated in Bug Q235601 in the MSDN, this is actually a 
bug from Microsoft, so we can't do much to correct it:

http://support.microsoft.com/directory/article.asp?ID=KB;EN-
US;Q235601&LN=EN-US

It may be possible to going around and clean behind the 
scenes in the _environ C array, but that may be (extremely) 
brittle.

Logged In: YES 
user_id=7549

Um, like what source file is this in?