Changeset 4552

Timestamp:

Jul 5, 2013 8:14:14 AM (11 years ago)

Author:

nanang

Message:

Fix #1686: release mutex when invoking callback to avoid deadlock. Also a bit memory usage optimization, i.e: avoid bloated pool by unfreed old/expired packet in cache entry.

File:

• pjproject/trunk/pjlib-util/src/pjlib-util/resolver.c (modified) (7 diffs)

pjproject/trunk/pjlib-util/src/pjlib-util/resolver.c

@@ -158 +158 @@
     pj_time_val              expiry_time;   /**< Expiration time.           */
     pj_dns_parsed_packet    *pkt;           /**< The response packet.       */
+    unsigned                 ref_cnt;       /**< Reference counter.         */
 };
 
@@ -701 +702 @@
     cache = PJ_POOL_ZALLOC_T(pool, struct cached_res);
     cache->pool = pool;
+    cache->ref_cnt = 1;
 
     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;
 }
 
@@ -776 +796 @@
             status = PJ_STATUS_FROM_DNS_RCODE(status);
 
+            /* Workaround for deadlock problem. Need to increment the cache's
+             * ref counter first before releasing mutex, so the cache won't be
+             * destroyed by other thread while in callback.
+             */
+            cache->ref_cnt++;
+            pj_mutex_unlock(resolver->mutex);
+
             /* This cached response is still valid. Just return this
              * response to caller.
@@ -784 +811 @@
 
             /* Done. No host resolution is necessary */
+            pj_mutex_lock(resolver->mutex);
+
+            /* Decrement the ref counter. Also check if it is time to free
+             * the cache (as it has been expired).
+             */
+            cache->ref_cnt--;
+            if (cache->ref_cnt <= 0)
+                free_entry(resolver, cache);
 
             /* Must return PJ_SUCCESS */
@@ -796 +831 @@
         pj_hash_set(NULL, resolver->hrescache, &key, sizeof(key), 0, NULL);
 
-        /* Store the entry into free nodes */
-        free_entry(resolver, cache);
+        /* Also free the cache, if it is not being used (by callback). */
+        cache->ref_cnt--;
+        if (cache->ref_cnt <= 0)
+            free_entry(resolver, cache);
 
         /* Must continue with creating a query now */
@@ -1208 +1245 @@
     if (cache == NULL) {
         cache = alloc_entry(resolver);
+    } else {
+        /* Reset cache to avoid bloated cache pool */
+        reset_entry(&cache);
     }
 
@@ -1216 +1256 @@
      * the name being requested.
      */
-    cache->pkt = NULL;
     pj_dns_packet_dup(cache->pool, pkt, 
                       PJ_DNS_NO_NS | PJ_DNS_NO_AR,