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

Re: Non-thread-safe hash iterator

From: David James <james82_at_gmail.com>
Date: Thu, 8 Oct 2009 13:45:26 -0700

On Thu, Oct 8, 2009 at 7:16 AM, Julian Foad <julianfoad_at_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
Received on 2009-10-08 22:46:09 CEST

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