Re: apr_hash_overlay returns hash with duplicate keys

On 10 December 2015 at 20:20, Julian Foad <julianfoad_at_apache.org> wrote:
> Ivan Zhakov wrote:
>> On 10 December 2015 at 19:14, Julian Foad <julianfoad_at_apache.org> wrote:
>>> APR devs, Subversion devs:
>>>
>>> On Subversion's Mac OS buildbots it appears that apr_hash_overlay()
>>> sometimes returns a hash containing duplicate keys, which (as I
>>> understand it) should be impossible.
>>>
>>> We had an issue where some 'svnmover' tests were failing only on Mac
>>> OS buildbots. I added some debugging in Subversion commits r1719056,
>>> r1719067, r1719072, r1719074.
>>>
>>> Buildbot result:
>>> https://ci.apache.org/builders/svn-x64-macosx-bdb/builds/485/steps/Test%20ra_svn%2Bbdb
>>> --> debug output in 'faillog' shows duplicate keys in hash:
>>> "union_children={A, iota, foo, boozle, boozle, iota}"
>>>
>>> I replaced apr_hash_overlay() with my own simple re-implementation:
>>>
>>> http://svn.apache.org/r1719089 -- re-implement hash overlay
>>>
>> Hi Julian,
>>
>> That could be possible if two hashes uses different hash functions.
>> This could happen if you're using svn_hash__make() directly or
>> indirectly: for example RA get_dirent for svn:// protocol returns hash
>> with non-standard hash-function.
>
> Ugh. Yes, that is probably the cause. Thanks.
>
> I can see now that the doc string of apr_hash_overlay() does say "Both
> hash tables must use the same hash function" but that was not obvious,
> and I had totally forgotten that our Subversion code was using more
> than one hash function.
>
> I will use my own hash overlay function from now on.
>
I don't think this is proper fix for this problem. I think returning
hash with non-standard hash function from public API is a bug (and API
regression). Other API users may get to the same situation. So proper
fix would be revert these optimizations from public API imo.

-- 
Ivan Zhakov