<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/9951">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Approved for Submit

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">pbx_dundi.c: Misc memory management fixes when destroying peers<br><br>* In destroy_peer(), fixed memory leaks of lookup history strings and<br>qualify transactions when destroying peers.<br><br>* In destroy_peer(), fixed leaving the registerexpire scheduled callback<br>active when a peer is destroyed on a reload.  The reload marks and sweeps<br>peers so any peers not explicitly configured get destroyed.  Peers created<br>dynamically from the '*' peer will not exist until they re-register after<br>the reload.  These destroyed peers caused memory corruption when the<br>registerexpire timer expired.<br><br>* Made build_peer() not schedule any callbacks on the '*' peer<br>(empty_eid).  It is a special peer that is cloned to dynamically created<br>peers so it doesn't actually get involved in any message transactions.<br><br>* Made do_register_expire() remove the dundi/dpeers AstDB entry when a<br>peer registration expires.<br><br>* Fix deep_copy_peer() to not copy some things that cannot be copied to<br>the cloned peer structure.  Timers, message transactions, and lookup<br>history are specific to a peer instance.<br><br>* Made set_config() lock around processing the mappings configuration.<br><br>* Reordered unload_module() to handle load_module() declining the load due<br>to error.<br><br>Change-Id: Ib846b2b60d027f3a2c2b3b563d9a83a357dce1d6<br>---<br>M pbx/pbx_dundi.c<br>1 file changed, 54 insertions(+), 16 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/pbx/pbx_dundi.c b/pbx/pbx_dundi.c</span><br><span>index f8cfa72..6dab9b4 100644</span><br><span>--- a/pbx/pbx_dundi.c</span><br><span>+++ b/pbx/pbx_dundi.c</span><br><span>@@ -1318,7 +1318,9 @@</span><br><span> {</span><br><span>       struct dundi_peer *peer = (struct dundi_peer *)data;</span><br><span>         char eid_str[20];</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>  ast_debug(1, "Register expired for '%s'\n", ast_eid_to_str(eid_str, sizeof(eid_str), &peer->eid));</span><br><span style="color: hsl(120, 100%, 40%);">+   ast_db_del("dundi/dpeers", dundi_eid_to_str_short(eid_str, sizeof(eid_str), &peer->eid));</span><br><span>   peer->registerexpire = -1;</span><br><span>        peer->lastms = 0;</span><br><span>         memset(&peer->addr, 0, sizeof(peer->addr));</span><br><span>@@ -1540,7 +1542,18 @@</span><br><span> {</span><br><span>  struct permission *cur, *perm;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      memcpy(peer_dst, peer_src, sizeof(*peer_dst));</span><br><span style="color: hsl(120, 100%, 40%);">+        *peer_dst = *peer_src;</span><br><span style="color: hsl(120, 100%, 40%);">+        AST_LIST_NEXT(peer_dst, list) = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+       /* Scheduled items cannot go with the copy */</span><br><span style="color: hsl(120, 100%, 40%);">+ peer_dst->registerid = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ peer_dst->qualifyid = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+  peer_dst->registerexpire = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* Transactions and lookup history cannot go with the copy either */</span><br><span style="color: hsl(120, 100%, 40%);">+  peer_dst->regtrans = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ peer_dst->qualtrans = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+        memset(&peer_dst->lookups, 0, sizeof(peer_dst->lookups));</span><br><span> </span><br><span>      memset(&peer_dst->permit, 0, sizeof(peer_dst->permit));</span><br><span>    memset(&peer_dst->include, 0, sizeof(peer_dst->permit));</span><br><span>@@ -2367,8 +2380,7 @@</span><br><span>           AST_LIST_LOCK(&peers);</span><br><span>           AST_LIST_TRAVERSE(&peers, p, list) {</span><br><span>                     for (x = 0;x < DUNDI_TIMING_HISTORY; x++) {</span><br><span style="color: hsl(0, 100%, 40%);">-                          if (p->lookups[x])</span><br><span style="color: hsl(0, 100%, 40%);">-                                   ast_free(p->lookups[x]);</span><br><span style="color: hsl(120, 100%, 40%);">+                           ast_free(p->lookups[x]);</span><br><span>                          p->lookups[x] = NULL;</span><br><span>                             p->lookuptimes[x] = 0;</span><br><span>                    }</span><br><span>@@ -3180,8 +3192,7 @@</span><br><span>                                    if (!ast_eid_cmp(&trans->them_eid, &peer->eid)) {</span><br><span>                                              peer->avgms = 0;</span><br><span>                                          cnt = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-                                                if (peer->lookups[DUNDI_TIMING_HISTORY-1])</span><br><span style="color: hsl(0, 100%, 40%);">-                                                   ast_free(peer->lookups[DUNDI_TIMING_HISTORY-1]);</span><br><span style="color: hsl(120, 100%, 40%);">+                                           ast_free(peer->lookups[DUNDI_TIMING_HISTORY - 1]);</span><br><span>                                                for (x=DUNDI_TIMING_HISTORY-1;x>0;x--) {</span><br><span>                                                  peer->lookuptimes[x] = peer->lookuptimes[x-1];</span><br><span>                                                         peer->lookups[x] = peer->lookups[x-1];</span><br><span>@@ -4327,19 +4338,31 @@</span><br><span> </span><br><span> static void destroy_peer(struct dundi_peer *peer)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+  int idx;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    AST_SCHED_DEL(sched, peer->registerexpire);</span><br><span>       AST_SCHED_DEL(sched, peer->registerid);</span><br><span style="color: hsl(0, 100%, 40%);">-      if (peer->regtrans)</span><br><span style="color: hsl(120, 100%, 40%);">+        if (peer->regtrans) {</span><br><span>             destroy_trans(peer->regtrans, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+  }</span><br><span>    AST_SCHED_DEL(sched, peer->qualifyid);</span><br><span style="color: hsl(120, 100%, 40%);">+     if (peer->qualtrans) {</span><br><span style="color: hsl(120, 100%, 40%);">+             destroy_trans(peer->qualtrans, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span>    destroy_permissions(&peer->permit);</span><br><span>   destroy_permissions(&peer->include);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Release lookup history */</span><br><span style="color: hsl(120, 100%, 40%);">+  for (idx = 0; idx < ARRAY_LEN(peer->lookups); ++idx) {</span><br><span style="color: hsl(120, 100%, 40%);">+          ast_free(peer->lookups[idx]);</span><br><span style="color: hsl(120, 100%, 40%);">+      }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>  ast_free(peer);</span><br><span> }</span><br><span> </span><br><span> static void destroy_map(struct dundi_mapping *map)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-     if (map->weightstr)</span><br><span style="color: hsl(0, 100%, 40%);">-          ast_free(map->weightstr);</span><br><span style="color: hsl(120, 100%, 40%);">+  ast_free(map->weightstr);</span><br><span>         ast_free(map);</span><br><span> }</span><br><span> </span><br><span>@@ -4683,10 +4706,11 @@</span><br><span>            ast_log(LOG_WARNING, "Peer '%s' is supposed to have permission for some inbound searches but isn't an inbound peer or outbound precache!\n",</span><br><span>                   ast_eid_to_str(eid_str, sizeof(eid_str), &peer->eid));</span><br><span>        } else {</span><br><span style="color: hsl(0, 100%, 40%);">-                if (needregister) {</span><br><span style="color: hsl(0, 100%, 40%);">-                     peer->registerid = ast_sched_add(sched, 2000, do_register, peer);</span><br><span style="color: hsl(0, 100%, 40%);">-            }</span><br><span>            if (ast_eid_cmp(&peer->eid, &empty_eid)) {</span><br><span style="color: hsl(120, 100%, 40%);">+                 /* Schedule any items for explicitly configured peers. */</span><br><span style="color: hsl(120, 100%, 40%);">+                     if (needregister) {</span><br><span style="color: hsl(120, 100%, 40%);">+                           peer->registerid = ast_sched_add(sched, 2000, do_register, peer);</span><br><span style="color: hsl(120, 100%, 40%);">+                  }</span><br><span>                    qualify_peer(peer, 1);</span><br><span>               }</span><br><span>    }</span><br><span>@@ -4925,13 +4949,17 @@</span><br><span>          v = v->next;</span><br><span>      }</span><br><span>    AST_LIST_UNLOCK(&peers);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>       mark_mappings();</span><br><span>     v = ast_variable_browse(cfg, "mappings");</span><br><span style="color: hsl(0, 100%, 40%);">-     while(v) {</span><br><span style="color: hsl(120, 100%, 40%);">+    while (v) {</span><br><span style="color: hsl(120, 100%, 40%);">+           AST_LIST_LOCK(&peers);</span><br><span>           build_mapping(v->name, v->value);</span><br><span style="color: hsl(120, 100%, 40%);">+               AST_LIST_UNLOCK(&peers);</span><br><span>                 v = v->next;</span><br><span>      }</span><br><span>    prune_mappings();</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>  mark_peers();</span><br><span>        cat = ast_category_browse(cfg, NULL);</span><br><span>        while(cat) {</span><br><span>@@ -4948,6 +4976,7 @@</span><br><span>                 cat = ast_category_browse(cfg, cat);</span><br><span>         }</span><br><span>    prune_peers();</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>     ast_config_destroy(cfg);</span><br><span>     load_password();</span><br><span>     if (globalpcmodel & DUNDI_MODEL_OUTBOUND)</span><br><span>@@ -4980,15 +5009,24 @@</span><br><span>              pthread_join(previous_clearcachethreadid, NULL);</span><br><span>     }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   close(netsocket);</span><br><span style="color: hsl(0, 100%, 40%);">-       io_context_destroy(io);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>      mark_mappings();</span><br><span>     prune_mappings();</span><br><span>    mark_peers();</span><br><span>        prune_peers();</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      ast_sched_context_destroy(sched);</span><br><span style="color: hsl(120, 100%, 40%);">+     if (-1 < netsocket) {</span><br><span style="color: hsl(120, 100%, 40%);">+              close(netsocket);</span><br><span style="color: hsl(120, 100%, 40%);">+             netsocket = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+       }</span><br><span style="color: hsl(120, 100%, 40%);">+     if (io) {</span><br><span style="color: hsl(120, 100%, 40%);">+             io_context_destroy(io);</span><br><span style="color: hsl(120, 100%, 40%);">+               io = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+    }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   if (sched) {</span><br><span style="color: hsl(120, 100%, 40%);">+          ast_sched_context_destroy(sched);</span><br><span style="color: hsl(120, 100%, 40%);">+             sched = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span>        return 0;</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/9951">change 9951</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/9951"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ib846b2b60d027f3a2c2b3b563d9a83a357dce1d6 </div>
<div style="display:none"> Gerrit-Change-Number: 9951 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>