Menu
▾
▴
#1470 Win2000, putenv, not freeing memory
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.
Discussion
Logged In: YES
user_id=7549
Um, like what source file is this in?
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
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=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=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?
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=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.tcl 9 Jul 2001 23:05:19 -0000 1.43
--- doc.tcl 27 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
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
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
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=240
I am seeing this on windows XP as well, just FYI.
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
#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.
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=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.
suggested fix (release candidate)
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.