Bug 485090

Created attachment 358137 [details]
Patch for daemon_core memory leak relating to soap_init

When traversing the documentation for gsoap I had noticed that initialization differs for heap allocated objects.  Excerpt from page 17-18:
My Comments >>>>>:

For example, the following code stack-allocates the runtime
environment which is used for multiple remote method calls:

      int main()
      {
        struct soap soap;
        ...
        soap init(&soap); // initialize runtime environment
        ...
                                                       17
       soap  call ns method1(&soap, ...); // make a remote call
       ...
             call ns method2(&soap, ...); // make another remote call
       soap
       ...
             destroy(&soap); // remove deserialized class instances (C++ only)
       soap
       soap  end(&soap); // clean up and remove deserialized data
       soap  done(&soap); // detach environment (last use and no longer in scope)
       ...
     }

The runtime environment can also be heap allocated:
>>>>: This is the interesting point, when changed, memory leaks are cleaned up

     int main()
     {
       struct soap *soap;
       ...
       soap = soap new(); // allocate and initialize runtime environment
       if (!soap) // couldn’t allocate: stop
       ...
       soap call ns method1(soap, ...); // make a remote call
       ...
       soap call ns method2(soap, ...); // make another remote call
       ...
       soap destroy(soap); // remove deserialized class instances (C++ only)
       soap end(soap); // clean up and remove deserialized data
       soap free(soap); // detach and free runtime environment
     }

The use of soap_destroy/end/free looks good as well as soap_new instead of new. The reconfig still needs to call init_soap(), which is defined in soap_core.cpp. It does all the meaningful setup.

Actually, the original code looks to never re-create the soap struct on reconfig, which is wrong because it forces a restart to change any soap specific config.

Created attachment 358160 [details]
Updated patch per matts comments

I had talked with some folks here, and updated the patch per matts previous comments.  

Likely fixes a couple of potential bugs, due to the reconfig.

Looks good. I'd add a comment near the "//soap_init(soap);" explaining why it is commented out and shouldn't be done anymore.

http://condor-wiki.cs.wisc.edu/index.cgi/tktview?tn=686

Fix should be in 7.4.0-0.1

Tested on RHEL5.4 condor-7.4.0-0.5 i386/x86_64 and RHEL4.8 condor-7.4.0-0.4 i386/x86_64 and it works without memory leaks(valgrind result) --> VERIFIED

Release note added. If any revisions are required, please set the 
"requires_release_notes" flag to "?" and edit the "Release Notes" field accordingly.
All revisions will be proofread by the Engineering Content Services team.

New Contents:
please see bug summary.

Can someone please verify my interpretation of this bug?

Thanks,
LKB

Release note updated. If any revisions are required, please set the 
"requires_release_notes"  flag to "?" and edit the "Release Notes" field accordingly.
All revisions will be proofread by the Engineering Content Services team.

Diffed Contents:
@@ -1 +1,8 @@
-please see bug summary.+Grid bug fix
+
+C: A daemon is reconfigured
+C: A memory leak appeared in SOAP
+F: SOAP is now initialized in a different way
+R: The memory leak no longer appears
+
+NEEDS FACT CHECKING BEFORE ADDING TO RELNOTES

From IRC:

<tstclair> Lana, re 485090, yes that is correct
<Lana> tstclair: thanks

LKB

Release note updated. If any revisions are required, please set the 
"requires_release_notes"  flag to "?" and edit the "Release Notes" field accordingly.
All revisions will be proofread by the Engineering Content Services team.

Diffed Contents:
@@ -5,4 +5,4 @@
 F: SOAP is now initialized in a different way
 R: The memory leak no longer appears
 
-NEEDS FACT CHECKING BEFORE ADDING TO RELNOTES+When a daemon was reconfigured, a memory leak appeared in SOAP. SOAP is now initialized in a different way and the memory leak no longer appears.

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHEA-2009-1633.html