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

Re: svn commit: r1659013 - /subversion/trunk/subversion/libsvn_subr/dso.c

From: Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
Date: Thu, 12 Feb 2015 15:26:57 +0300

Philip Martin <philip_at_apache.org> writes:

> Wrap svn_dso_initialize2 call with svn_atomic__init_once, this
> fixes a crash in the DSO hash code when running the C tests in
> parallel.
>
> * subversion/libsvn_subr/dso.c
> (atomic_init_func): New.
> (svn_dso_load): Use svn_atomic__init_once.

I think we should also address this problem in our test suite. As I see, this
change avoids segfaults with --enable-runtime-module-search, but there is more
to it. Documentation for svn_dso_initialize2() states the following (by the
way, the "will not be entirely thread safe" part is now outdated, right?):

 * @note This should be called prior to the creation of any pool that
 * is passed to a function that comes from a DSO, otherwise you
 * risk having the DSO unloaded before all pool cleanup callbacks
 * that live in the DSO have been executed. If it is not called
 * prior to @c svn_dso_load being used for the first time there
 * will be a best effort attempt made to initialize the subsystem,
 * but it will not be entirely thread safe and it risks running
 * into the previously mentioned problems with DSO unloading and
 * pool cleanup callbacks.

I am not sure that taking some risks within the test suite is a good idea,
and I think that we should call functions like svn_dso_initialize2() and
svn_fs_initialize(). If we don't, everything might still work fine, but it
also might not, and we could spend a lot of time debugging random failures
if a test runs into one of these pitfalls happening due to the absense of
explicit initialization. We do call these initializers in mod_dav_svn, svn,
svnserve — so why not also do the same in the test programs?

I don't see a reason to use a custom way of bootstrapping things within
svn_test_main(), as opposed to main() functions in svn, svnadmin and other
command-line tools, and I attached a patch that brings this common approach
to svn_test_main(). Quick inspection shows a couple of other programs that
should probably do the same, e.g., atomic-ra-revprop-change. However, I
think this is less important, and that we could do it separately.

What do you think?

Regards,
Evgeny Kotkov

Received on 2015-02-12 13:28:31 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.