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
