On Wed, 18 Aug 2004, Philip Martin wrote:
> Philip Martin <philip@codematters.co.uk> writes:
>
> > "Peter N. Lundblad" <peter@famlundblad.se> writes:
> >
> >>> > + handle_pool = svn_pool_create (NULL);
> >>>
> >>> This creates a pool in every thread, but I don't see the pool getting
> >>> cleared or destroyed. An application that creates threads continually
> >>> (e.g. threaded svnserve) will cause the number of pools to grow
> >>> without bound.
> >>>
> >> xlate_cleanup will take care of that. It is called when the thread is
> >> destoyed.
> >
> > Unless you are planning to write some more code I don't understand.
> > xlate_cleanup appears to be applied to the pool passed to
> > svn_utf_initialize. That pool could well have a lifetime much longer
> > than the thread lifetime, typically I would expect the pool to last as
> > long as the application runs.
>
> I see now, it's a thread cleanup not a pool cleanup. That would work
> if APR were not broken as you point out below.
>
Then I understand your confusion...
> >> Problems is, when I check the implementation of
> >> apr_threadkey_private_create, the destructor is only used on POSIX and
> >> netware systems, not on Windows/OS2. So on these platforms, it is a leak
> >> after all. Maybe there is no way to register cleanups on these
> >> platforms,. This should have been documented in the APR headers!!!
>
> That does look broken, it should probably be brought up on the dev@apr
> list.
>
> Without a working thread destructor it will be very hard to use a
> thread private solution to solve the caching problem.
>
It will be hard to use thread-specific storage from a library in general,
or from an application when you don't have control of the thread
creation/termination. I don't know, however, if it is possible to get the
destructors called on Win32. The Win32 TLS doesn't have destructors.
POssibly you could solve it in DllMain if APR is linked dynamically.
Anyway, this is an APR question, not svn...
> >>
> >>> > + handle_hash = apr_hash_make (pool);
> >>> > + if (apr_threadkey_private_set (handle_hash, xlate_handle_key)
> >>>
> >>> If pool is cleared handle_hash will become invalid, but will still be
> >>> referenced by the thread private data. The next time the thread calls
> >>> get_xlate_handle it will crash, or do something worse.
> >>>
> >> When the pool is cleared, the thread is dying, so the TSD for that thread
> >> isn't used again. This would be true where the pool is destroyed at all...
> >
> > Again I don't understand. pool is an argument to one of the
> > svn_utf_xxx_(to/from)_xxx functions, it could well have a lifetime
> > much shorter than the thread and may well get repeatedly cleared.
>
> Did you mean to allocate handle_hash from handle_pool rather than
> pool? Then the hash would have the correct lifetime. It's all a bit
> academic if thread destructors don't work on win32.
>
Yes and yes. I noticed that error later.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 18 22:03:00 2004