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

Joe Orton wrote:
> On Fri, Jul 06, 2007 at 07:53:19PM +0200, Stefan Küng wrote:
>> Well, I'm not that familiar with the neon code, but from what I can see
>> it should be thread safe. But just to be sure, I've cc'ed Joe Orton. I
>> guess he's the one who could clear this issue up.
>>
>> Joe, is the ne_sock_exit() function thread-safe? If not, then when
>> should this be called?
>
> ne_sock_exit() destroys any process-global state created by
> ne_sock_init(), so no, it's not thread-safe. The OpenSSL locking
> callbacks are one case of such process-global state - the other case is
> in the use of the Win32 socket/SSPI libraries.

I'm sorry if I'm a little bit slow, but if you destroy process-global
states in ne_sock_exit(), why can't you call ERR_free_strings() there too?

> ne_sock_exit() should only be used (if at all) when you know there will
> be no subsequent use of neon within the process by the caller. AFAIK
> this isn't true in the ra_dav session cleanup, since there are two
> sessions and there will be another call to ne_session_destroy().
>
> (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?

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.

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