Bugzilla – Bug 1555
Memory leak and off by one error
Last modified: 2005-12-02 15:04:52
You need to log in before you can comment on or make changes to this bug.
This is a patch from David Smith at CERN. It has been tested as part of the VDT. It fixes two things in the GASS HTTP transfer code: 1) A memory leak 2) An off by one error --- bad/gass/transfer/source/library/globus_gass_transfer_http.c Mon Jan 5 20:48:11 2004 +++ fix/gass/transfer/source/library/globus_gass_transfer_http.c Wed Jan 21 19:29:05 2004 @@ -1966,6 +1966,9 @@ globus_l_gass_transfer_http_lock(); proto = (globus_gass_transfer_http_listener_proto_t *) arg; + if (proto->request->connected_subject != GLOBUS_NULL) + globus_free(proto->request->connected_subject); + proto->request->connected_subject = globus_libc_strdup(identity); switch(proto->request->authorization_mode) @@ -5485,7 +5488,7 @@ } if(input[i] == CR) { - if(i + 2 > max_to_scan) + if(i + 2 >= max_to_scan) { /* not enough data */ return GLOBUS_TRUE; -alain
Can you provide more information about what would trigger these bugs? joe
Joe, David may provide the details, but I can already say the following. Both problem were discovered by running the Condor-G "gahp_server" under the "valgrind" memory leak finder. 1. The memory leak seems fairly evident to me: if the pointer is non-null, it should first be freed, before it gets re-assigned on the next line. 2. The "valgrind" memory access checker actually reported an access to the element just after the end of the array, so it can happen! FYI, the "gahp_server" is a *long-lived* Globus program, of which there is an instance for each user submitting jobs through Condor-G. In EDG/LCG jobs are submitted via a central Resource Broker, which then may have many instances running in parallel, so all those leaks can add up to a huge number! This has actually been observed, and that is why David has spent a lot of time chasing all the memory and file descriptor leaks in the "gahp_server". So please give these carefully tested patches a serious consideration. Thanks!
I'm just trying to verify the case where the memory leak occurs. I'm not sure that the function should be called more than once per request---hence, I'm concerned this patch is masking some other bad behavior. joe
Hi Joe, I saw that the routine was being called twice per connection. Are you able to reproduce that? If not, I will send some more information - I can get the two locations from where it was called. Thanks, David
Hello, OK, I have the two places to hand so here they are: The first is: globus_l_io_write_auth_token() in globus_io_securesocket.c (line 2634) -> globus_l_io_securesocket_call_auth_callback() -> handle->securesocket_attr.auth_callback() [globus_l_gass_transfer_http_authorization_callback] and then globus_l_io_write_auth_token() in globus_io_securesocket.c (line 2649) -> init_info->callback() [globus_l_io_secure_accept_callback() in " (line 3072)] -> handle->securesocket_attr.auth_callback() [globus_l_gass_transfer_http_authorization_callback] (line numbers are from my copy of the source, so they may vary for you) David
I'm unable to duplicate the leak with the current CVS trunk (where the IO implementation is quite different than 2.4). I've committed the other part of the patch to 3.2 branch and trunk. joe
