Hi Ming,
sorry for the noise. Please disregard my last email.
That's still not the problem. I'll try to find the actual cause of my problem and get back to you then. In my defense, the problem is intermittent and I don't know how to reliably reproduce it yet.
- Lars
>> The entry update is done a few lines below the code patch you provided above:
>> pj_hash_set_np(resolver->hrescache, &cache->key, sizeof(*key), hval,
>> cache->hbuf, cache);
>
> Ah, ok. I missed that.
>
>> So I'm still not sure whether reset_entry() is the problem or it may be caused by something else.
>
> I have thought about it some more: reset_entry clears the cache entry's key. pj_hash_set_np then sorts it into the wrong place.
>
> In my bug-case that lead to a loop in the linked list of the hash table (next pointed to itself). The actual memory corruption happened much later at pjsua_shutdown when the memory of the hash table was reclaimed, and the same memory was freed twice due to the loop in the linked list.
>
> This patch fixes the problem and is much closer to your original intention:
>
> --- a/pjlib-util/src/pjlib-util/resolver.c
> +++ b/pjlib-util/src/pjlib-util/resolver.c
> @@ -710,10 +710,12 @@ static struct cached_res *alloc_entry(pj_dns_resolver *resolver)
> static void reset_entry(struct cached_res **p_cached)
> {
> pj_pool_t *pool;
> + struct res_key key;
> struct cached_res *cache = *p_cached;
> unsigned ref_cnt;
>
> pool = cache->pool;
> + key = cache->key;
> ref_cnt = cache->ref_cnt;
>
> pj_pool_reset(pool);
> @@ -721,6 +723,7 @@ static void reset_entry(struct cached_res **p_cached)
> cache = PJ_POOL_ZALLOC_T(pool, struct cached_res);
> cache->pool = pool;
> cache->ref_cnt = ref_cnt;
> + cache->key = key;
> *p_cached = cache;
> }
>
> Regards,
>
> - Lars
>
>>
>> On Wed, Aug 21, 2013 at 8:11 PM, Lars Immisch <lars at ibp.de> wrote:
>> Hi,
>>
>> I've seen a memory corruption problem in pjlib-util/resolver.c that was introduced with the changes for #1686 (http://trac.pjsip.org/repos/ticket/1686)
>>
>> This patch fixes the problem:
>>
>> --- a/pjlib-util/src/pjlib-util/resolver.c
>> +++ b/pjlib-util/src/pjlib-util/resolver.c
>> @@ -706,24 +706,6 @@ static struct cached_res *alloc_entry(pj_dns_resolver *resolver)
>> return cache;
>> }
>>
>> -/* Re-allocate cache entry, to free cached packet */
>> -static void reset_entry(struct cached_res **p_cached)
>> -{
>> - pj_pool_t *pool;
>> - struct cached_res *cache = *p_cached;
>> - unsigned ref_cnt;
>> -
>> - pool = cache->pool;
>> - ref_cnt = cache->ref_cnt;
>> -
>> - pj_pool_reset(pool);
>> -
>> - cache = PJ_POOL_ZALLOC_T(pool, struct cached_res);
>> - cache->pool = pool;
>> - cache->ref_cnt = ref_cnt;
>> - *p_cached = cache;
>> -}
>> -
>> /* Put unused/expired cached entry to the free list */
>> static void free_entry(pj_dns_resolver *resolver, struct cached_res *cache)
>> {
>> @@ -816,7 +798,7 @@ PJ_DEF(pj_status_t) pj_dns_resolver_start_query( pj_dns_resolver *resolver,
>> * the cache (as it has been expired).
>> */
>> cache->ref_cnt--;
>> - if (cache->ref_cnt <= 0)
>> + if (cache->ref_cnt == 0)
>> free_entry(resolver, cache);
>>
>> /* Must return PJ_SUCCESS */
>> @@ -1251,11 +1233,8 @@ static void update_res_cache(pj_dns_resolver *resolver,
>> */
>> cache->ref_cnt--;
>> cache = alloc_entry(resolver);
>> - } else {
>> - /* Reset cache to avoid bloated cache pool */
>> - reset_entry(&cache);
>> }
>> -
>> +
>> /* Duplicate the packet.
>> * We don't need to keep the NS and AR sections from the packet,
>> * so exclude from duplication. We do need to keep the Query
>> --
>>
>> (The second hunk is not strictly necessary, BTW, but ref_cnt is unsigned, so never < 0)
>>
>> I haven't looked at it very closely, but I believe the problem ist that while reset_entry reallocates the cache entry, it does not refresh the links in the hash table.
>>
>> There might be a better way to fix this and keep the intended memory usage optimization, but I didn't do it because the problem was hard to reproduce.
>>
>> - Lars
> _______________________________________________
> Visit our blog: http://blog.pjsip.org
>
> pjsip mailing list
> pjsip at lists.pjsip.org
> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 496 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.pjsip.org/pipermail/pjsip_lists.pjsip.org/attachments/20130822/c47395c2/attachment-0001.asc>
