On Wed, 27 Jul 2005, Greg Hudson murmured woefully:
> On Wed, 2005-07-27 at 17:34 +0100, Nix wrote:
>> The cause of this is that a key in the auth_baton hash has been
>> invalidated and points at a no-longer-valid address. How, you might ask,
>> since all the keys are manifest constants?
>
> Perhaps we should stop using a manifest constant, then. We've certainly
> chosen to allocate memory in a passed-in pool rather than use a manifest
> constant in some past situations, for just this reason.
>
> Can you say more about where these constants are coming from?
They are #defines which expand to string literals, such as
SVN_AUTH_PARAM_CONFIG, SVN_AUTH_PARAM_SERVER_GROUP, &c;
libsvn_ra_dav/session.c:svn_ra_dav__open() adds several of these via
svn_auth_set_parameter(), so there'll always be some in the baton as
long as an RA connection's been opened successfully. And the pool that
the baton is allocated in may have a longer lifespan than the pool that
the shared object was allocated in.
(I still wonder how this ever works on anyone's machine; perhaps a lot
of OSes tend to put repeatedly dlopen()ed libraries at the same place
every time?)
>> So as a possibly more maintainable alternative, the hack below fixes
>> apr_dso_load() callers to always use tiny dedicated pools which are
>> never deallocated.
>
> Your patch isn't very thread-safe.
It is thread-safe iff static allocation is thread-safe. Of course, the C
standard says nothing on this point; I can't see anything in POSIX,
either. Curses, I thought there was something.
We'd have to wrap a mutex around the pool initialization.
> We could use svn_ra_initialize() to
> set up the dedicated pool and a mutex protecting it, but because we
> introduced svn_ra_initialize() in 1.2, we can't require every client to
> call it.
Well, no. If old clients don't, they might crash. New ones should.
(Os isn't that good enough?)
> More importantly, it should be possible to dynamically load the
> Subversion client library and then unload it without leaving big
> footprints behind. So, we should get our memory management story right,
> rather than applying a big hammer to the DSO system.
I'm not sure if the underlying bug here is that the auth baton is
persisting for too long, or that the DAV library's getting unloaded when
it shouldn't be (why should a `svn ls' unload it only to load it again?)
or that the bug is that we should never use string literals in
dlopen()ed shared libraries...
... but the latter fix feels wrong somehow.
--
`Tor employs several thousand editors who they keep in dank
subterranean editing facilities not unlike Moria' -- James Nicoll
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 27 23:38:40 2005