Memory leak in SSL connections, CTX_free needed

There is a memory leak in SSL connections (both StartTLS and OldSSLClientIn)

The context object created for the ssl stream needs to be freed when the connection closes.

Net::SSLeay::CTX_free must be called on $ctx
just like
Net::SSLeay::free is called on $ssl

I've also done a tiny bit of consolidation of code shared between StartTLS and OldSSLClientIn around the finalizing of  SSL

-------

patch 3 committed in http://code.sixapart.com/trac/djabberd/changeset/821

[dangabrad, 2008-06-20 05:53:44]

Overall, nice.

Few nits.

http://codereview.appspot.com/2326/diff/1/3
File lib/DJabberd/Stanza/StartTLS.pm (right):

http://codereview.appspot.com/2326/diff/1/3#newcode51
Line 51: sub finalize_ssl_negotiation {
A little one-line or so comment above this function explaining when it's to be
called and what it does would be nice.

http://codereview.appspot.com/2326/diff/1/3#newcode54
Line 54: #Net::SSLeay::set_verify($ssl, Net::SSLeay::VERIFY_PEER(), 0);
Feel free to remove this.  I hate commented out code... not sure why it was left
in before.

http://codereview.appspot.com/2326/diff/1/3#newcode61
Line 61: Net::SSLeay::CTX_free($ctx);
I believe there's another bug report that we should really only have one global
CTX object around and reusing it, rather than making one per connection.  If we
did that, then we wouldn't have to free the context object all the time too.

Feel free to add a comment to that effect.

[Jacob, 2008-06-23 18:44:40]

Hi, I've uploaded a new patch

http://codereview.appspot.com/2326/diff/1/3
File lib/DJabberd/Stanza/StartTLS.pm (right):

http://codereview.appspot.com/2326/diff/1/3#newcode51
Line 51: sub finalize_ssl_negotiation {
On 2008/06/20 05:53:45, bradfitz wrote:
> A little one-line or so comment above this function explaining when it's to be
> called and what it does would be nice.

Done.

http://codereview.appspot.com/2326/diff/1/3#newcode54
Line 54: #Net::SSLeay::set_verify($ssl, Net::SSLeay::VERIFY_PEER(), 0);
On 2008/06/20 05:53:45, bradfitz wrote:
> Feel free to remove this.  I hate commented out code... not sure why it was
left
> in before.

Done.

http://codereview.appspot.com/2326/diff/1/3#newcode61
Line 61: Net::SSLeay::CTX_free($ctx);
On 2008/06/20 05:53:45, bradfitz wrote:
> I believe there's another bug report that we should really only have one
global
> CTX object around and reusing it, rather than making one per connection.  If
we
> did that, then we wouldn't have to free the context object all the time too.
> 
> Feel free to add a comment to that effect.

Done.

http://codereview.appspot.com/2326/diff/1/3#newcode71
Line 71: #Net::SSLeay::connect($ssl) or Net::SSLeay::die_now("Failed SSL connect
($!)");
also removing this commented out line