On 03.09.2012 22:11, Branko Čibej wrote:
> On 03.09.2012 20:47, Stefan Küng wrote:
>> Hi,
>>
>> It seems that the svn_config_t structure isn't thread safe, i.e. can't
>> be shared among multiple threads.
>> See here for a detailed report on what problems this causes:
>> http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=3003152
>>
>>
>> Is this by design? I assumed that all svn APIs are thread safe for
>> read access at least unless noted otherwise (example: the API
>> svn_utf_initialize states its thread-unsafeness in the note section of
>> the docs).
>
> Subversion's functions are re-entrant. Data structures, including
> svn_config_t, are thread-specific. We don't promise any more than that.
>
>> While we can work around that thread problem a little bit, that
>> workaround has its own big problems:
>> * each thread would need its own svn_config_t structure
>> --> for each thread the config file is read (open, read, close)
>> which results in multiple unnecessary disk accesses
>
> One would assume that the config file is contents are cached once
> they've been read, so disk accesses shouldn't be an issue.
>
>> --> when the config file is read multiple times, sometimes the read
>> fails (at least on Windows) because the file is opened already -
>> usually because virus scanners open and scan every file that a
>> process accesses, even when only for reading.
>
> So we have to add a delay loop. Why a file that has not been changes
> should be locked by virus scanners is, IMO, an issue to be taken up with
> Windows virus scanner writers.
Well, in an ideal world that would work. But that's not how it works: we
have to deal with such issues ourselves.
> Also, one can always use a mutex to control access to the svn_config_t.
> Surely that's preferable to re-reading it any number of times.
>
>> Joel Jirak made a patch for subversion/libsvn_subr/config.c which
>> would fix the problem (see the thread linked above). Would that be an
>> acceptable solution?
>
> The patch creates a new pool for almost every access, so no, that's not
> acceptable. The only way to make this work would be to rev all the
> svn_config_* global APIs to accept a scratch pool argument from which
> allocations should be made.
>
> Either that, or add a new API that creates a deep-copy of the in-memory
> svn_config_t structure, making this another way to avoid re-reading the
> config file for each thread and avoiding the hassle of managing a mutex.
I'm in favor of implementing a deep copy API for this. I think that
would be the best solution.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Received on 2012-09-03 22:17:14 CEST