[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] cleanup the neon socket when closing the ra_session

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: 2007-07-09 21:57:07 CEST

Joe Orton wrote:
> On Fri, Jul 06, 2007 at 10:16:05PM +0200, Stefan Küng wrote:
>> Joe Orton wrote:
> ...
>>> (Looking at the code, ra_dav should really just call ne_sock_init() once
>>> in svn_ra_dav__init, I think; it's unnecessary/wrong to call it each
>>> time an RA session is opened.)
>> Hmmm - then ne_sock_init() really initializes more than just a socket?
>> Maybe the function should be renamed to ne_global_init() or something
>> like that to avoid confusion?
>
> I'd have hoped that reading the documentation would make some of that
> confusion go away :) In retrospect, yes, _global_init() would make more
> sense; not sure if it's worth renaming at this point.

IMHO you should rename it. Just use a define like this:
#define ne_sock_init()(x) ne_global_init(x)
to keep it compatible. And then add a comment in the header file telling
people that ne_sock_init() is deprecated. If you don't rename it now,
then it will never happen and users will keep using it wrong.

>> The reason I got confused here is because I always got a lot of memory
>> leaks as soon as I access a repository via http/https (i.e., when neon
>> is used). First I thought it was because ne_sock_exit() isn't called for
>> every ne_sock_init(), but that didn't solve the leaks. Digging a little
>> bit further I discovered that OpenSSL isn't shut down at all, and I had
>> to do that myself before exiting my app.
>> Maybe that should be documented somewhere too? And AFAIK the Subversion
>> client doesn't shut down OpenSSL either - it should call:
>> ERR_free_strings();
>> EVP_cleanup();
>> CRYPTO_cleanup_all_ex_data();
>> before exiting (of course, only if it's linked against OpenSSL). Even if
>> OpenSSL wasn't initialized (e.g., because no network connection was
>> needed), those cleanup calls won't hurt because they just won't do anything.
>
> Unless this stuff has been fixed in OpenSSL recently, those are all
> either irreversible operations or are not ref-counted and will
> potentially break any other code which uses OpenSSL within the process.
> Doing it at application level is basically all that is feasible.

That's why I suggested that the Subversion command line client should do
that before exiting. A lot of people are using the command line client
code as an example on how to use the Subversion library, and if it's not
in there, they won't do it either.

> Really, my best advice is "don't use OpenSSL in threaded application" -
> see also neon's GnuTLS support.

Well, I can't do that. The users of TortoiseSVN would get really mad if
I would stop using OpenSSL. And yes, TortoiseSVN is threaded, in fact
all networking is done on a separate thread.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Jul 9 21:56:52 2007

This is an archived mail posted to the Subversion Dev mailing list.