Hi 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);
So I'm still not sure whether reset_entry() is the problem or it may be
caused by something else.
Regards,
Ming
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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.pjsip.org/pipermail/pjsip_lists.pjsip.org/attachments/20130822/843f75de/attachment-0001.html>
