You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2009/10/08 14:16:01 UTC
Re: Non-thread-safe hash iterator
I (Julian Foad) wrote:
> After applying the attached patch, the following instances would remain:
>
> [[[
> $ find subversion/ -name '*.[ch]' | xargs grep -n "apr_hash_first.*NULL"
> subversion/libsvn_subr/hash.c:502: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi))
> subversion/libsvn_client/merge.c:6700: for (hi = apr_hash_first(NULL, subtrees);
> subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c:507: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi))
> subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c:755: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi))
> subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c:856: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi))
> subversion/bindings/swig/ruby/libsvn_swig_ruby/swigutil_rb.c:1408: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi)) {
> subversion/bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c:240: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi)) {
> ]]]
>
> The ones in merge.c and hash.c I can easily fix too by passing a pool to
> those functions.
I applied the patch in r39869 and fixed the ones in merge.c and hash.c
in r39873.
> The ones in swigutil_*.c I'm not sure about, as they seem to be public
> functions. Do they need this treatment?
I don't know whether to touch those, not being familiar with SWIG.
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405025
Re: Non-thread-safe hash iterator
Posted by David James <ja...@gmail.com>.
On Thu, Oct 8, 2009 at 7:16 AM, Julian Foad <ju...@btopenworld.com> wrote:
> I (Julian Foad) wrote:
>> After applying the attached patch, the following instances would remain:
>>
>> [[[
>> $ find subversion/ -name '*.[ch]' | xargs grep -n "apr_hash_first.*NULL"
>> subversion/libsvn_subr/hash.c:502: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi))
>> subversion/libsvn_client/merge.c:6700: for (hi = apr_hash_first(NULL, subtrees);
>> subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c:507: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi))
>> subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c:755: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi))
>> subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c:856: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi))
>> subversion/bindings/swig/ruby/libsvn_swig_ruby/swigutil_rb.c:1408: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi)) {
>> subversion/bindings/swig/perl/libsvn_swig_perl/swigutil_pl.c:240: for (hi = apr_hash_first(NULL, hash); hi; hi = apr_hash_next(hi)) {
>> ]]]
>>
>> The ones in merge.c and hash.c I can easily fix too by passing a pool to
>> those functions.
>
> I applied the patch in r39869 and fixed the ones in merge.c and hash.c
> in r39873.
>
>> The ones in swigutil_*.c I'm not sure about, as they seem to be public
>> functions. Do they need this treatment?
>
> I don't know whether to touch those, not being familiar with SWIG.
Hi Julian,
I don't know what the threading policy of the Ruby or Perl bindings
are, but I can speak about the Python bindings.
The functions in swigutil_py.c aren't public functions, unless they
are explicitly exposed in a swig wrapper -- they generally are just
used internally by the SWIG bindings. The three functions you
mentioned (convert_hash, svn_swig_py_locationhash_to_dict, and
svn_swig_py_changed_path_hash_to_dict) are internal functions and are
not used directly by users of the bindings.
These three functions all assume that the the global interpreter lock
is being held prior to calling them, so I believe they do not need to
worry about threading. That said, if you want, you are welcome to
update them to use pools as a precautionary measure. (You probably
won't want to though, since you'll need to learn about SWIG to do
this.)
Cheers,
David
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405308