Re: svn_config_t is not thread-safe

On 03.09.2012 17:55, Stefan Fuhrmann wrote:
>
>
> On Mon, Sep 3, 2012 at 2:19 PM, Joel Jirak <joel_at_jirak.us
> <mailto:joel_at_jirak.us>> wrote:
>
> Here's a patch that will fix the problem. It works by un-singletoning
> SVNConfig, so to speak. I'm not entirely satisfied with it because
> SVNConfig then has to be created many more times than necessary. It's
> more of a proof-of-concept, really.
>
> My initial attemp was to change SVNConfig::GetConfig() to return a
> copy of the config via apr_hash_copy, but this didn't work, presumably
> because apr_hash_copy() does a shallow copy. I think this direction
> is the cleaner way to go. Perhaps a true clone would work, but I
> don't have time to investigate this now.
>
>
> Hi Joel,
>
> Thanks for the analysis and sending a patch!
> A quick review of the latter made me wonder:
>
> --- SVN/SVNInfo.cpp (revision 23251)
> +++ SVN/SVNInfo.cpp (working copy)
> @@ -68,7 +68,7 @@
>
> #ifdef _MFC_VER
> // set up the configuration
> - m_pctx->config = SVNConfig::Instance().GetConfig();
> + m_pctx->config = (new SVNConfig())->GetConfig();
>
>
> When will these two objects (SVNConfig and svn_config_t)
> be deleted?

They're not. He mentioned this is only a proof-of-concept.
Better would be to create a new svn_config_t object in a pool. Like the
refresh function does.
Or create a deep copy of the already populated svn_config_t object and
return that - that would also avoid reading the config file every time
(it's actually worse: not just the config file is read every time, the
registry as well).

Even better would be if the svn library would be changed to be thread safe.

I've sent an email to the svn dev list about this:
http://thread.gmane.org/gmane.comp.version-control.subversion.devel/137457

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=3003239
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].