[Asterisk-code-review] pbx dundi.c: Misc memory management fixes when destroying peers (asterisk[15])
Joshua Colp
asteriskteam at digium.com
Tue Aug 21 06:55:15 CDT 2018
Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/9951 )
Change subject: pbx_dundi.c: Misc memory management fixes when destroying peers
......................................................................
pbx_dundi.c: Misc memory management fixes when destroying peers
* In destroy_peer(), fixed memory leaks of lookup history strings and
qualify transactions when destroying peers.
* In destroy_peer(), fixed leaving the registerexpire scheduled callback
active when a peer is destroyed on a reload. The reload marks and sweeps
peers so any peers not explicitly configured get destroyed. Peers created
dynamically from the '*' peer will not exist until they re-register after
the reload. These destroyed peers caused memory corruption when the
registerexpire timer expired.
* Made build_peer() not schedule any callbacks on the '*' peer
(empty_eid). It is a special peer that is cloned to dynamically created
peers so it doesn't actually get involved in any message transactions.
* Made do_register_expire() remove the dundi/dpeers AstDB entry when a
peer registration expires.
* Fix deep_copy_peer() to not copy some things that cannot be copied to
the cloned peer structure. Timers, message transactions, and lookup
history are specific to a peer instance.
* Made set_config() lock around processing the mappings configuration.
* Reordered unload_module() to handle load_module() declining the load due
to error.
Change-Id: Ib846b2b60d027f3a2c2b3b563d9a83a357dce1d6
---
M pbx/pbx_dundi.c
1 file changed, 54 insertions(+), 16 deletions(-)
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
diff --git a/pbx/pbx_dundi.c b/pbx/pbx_dundi.c
index f8cfa72..6dab9b4 100644
--- a/pbx/pbx_dundi.c
+++ b/pbx/pbx_dundi.c
@@ -1318,7 +1318,9 @@
{
struct dundi_peer *peer = (struct dundi_peer *)data;
char eid_str[20];
+
ast_debug(1, "Register expired for '%s'\n", ast_eid_to_str(eid_str, sizeof(eid_str), &peer->eid));
+ ast_db_del("dundi/dpeers", dundi_eid_to_str_short(eid_str, sizeof(eid_str), &peer->eid));
peer->registerexpire = -1;
peer->lastms = 0;
memset(&peer->addr, 0, sizeof(peer->addr));
@@ -1540,7 +1542,18 @@
{
struct permission *cur, *perm;
- memcpy(peer_dst, peer_src, sizeof(*peer_dst));
+ *peer_dst = *peer_src;
+ AST_LIST_NEXT(peer_dst, list) = NULL;
+
+ /* Scheduled items cannot go with the copy */
+ peer_dst->registerid = -1;
+ peer_dst->qualifyid = -1;
+ peer_dst->registerexpire = -1;
+
+ /* Transactions and lookup history cannot go with the copy either */
+ peer_dst->regtrans = NULL;
+ peer_dst->qualtrans = NULL;
+ memset(&peer_dst->lookups, 0, sizeof(peer_dst->lookups));
memset(&peer_dst->permit, 0, sizeof(peer_dst->permit));
memset(&peer_dst->include, 0, sizeof(peer_dst->permit));
@@ -2367,8 +2380,7 @@
AST_LIST_LOCK(&peers);
AST_LIST_TRAVERSE(&peers, p, list) {
for (x = 0;x < DUNDI_TIMING_HISTORY; x++) {
- if (p->lookups[x])
- ast_free(p->lookups[x]);
+ ast_free(p->lookups[x]);
p->lookups[x] = NULL;
p->lookuptimes[x] = 0;
}
@@ -3180,8 +3192,7 @@
if (!ast_eid_cmp(&trans->them_eid, &peer->eid)) {
peer->avgms = 0;
cnt = 0;
- if (peer->lookups[DUNDI_TIMING_HISTORY-1])
- ast_free(peer->lookups[DUNDI_TIMING_HISTORY-1]);
+ ast_free(peer->lookups[DUNDI_TIMING_HISTORY - 1]);
for (x=DUNDI_TIMING_HISTORY-1;x>0;x--) {
peer->lookuptimes[x] = peer->lookuptimes[x-1];
peer->lookups[x] = peer->lookups[x-1];
@@ -4327,19 +4338,31 @@
static void destroy_peer(struct dundi_peer *peer)
{
+ int idx;
+
+ AST_SCHED_DEL(sched, peer->registerexpire);
AST_SCHED_DEL(sched, peer->registerid);
- if (peer->regtrans)
+ if (peer->regtrans) {
destroy_trans(peer->regtrans, 0);
+ }
AST_SCHED_DEL(sched, peer->qualifyid);
+ if (peer->qualtrans) {
+ destroy_trans(peer->qualtrans, 0);
+ }
destroy_permissions(&peer->permit);
destroy_permissions(&peer->include);
+
+ /* Release lookup history */
+ for (idx = 0; idx < ARRAY_LEN(peer->lookups); ++idx) {
+ ast_free(peer->lookups[idx]);
+ }
+
ast_free(peer);
}
static void destroy_map(struct dundi_mapping *map)
{
- if (map->weightstr)
- ast_free(map->weightstr);
+ ast_free(map->weightstr);
ast_free(map);
}
@@ -4683,10 +4706,11 @@
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",
ast_eid_to_str(eid_str, sizeof(eid_str), &peer->eid));
} else {
- if (needregister) {
- peer->registerid = ast_sched_add(sched, 2000, do_register, peer);
- }
if (ast_eid_cmp(&peer->eid, &empty_eid)) {
+ /* Schedule any items for explicitly configured peers. */
+ if (needregister) {
+ peer->registerid = ast_sched_add(sched, 2000, do_register, peer);
+ }
qualify_peer(peer, 1);
}
}
@@ -4925,13 +4949,17 @@
v = v->next;
}
AST_LIST_UNLOCK(&peers);
+
mark_mappings();
v = ast_variable_browse(cfg, "mappings");
- while(v) {
+ while (v) {
+ AST_LIST_LOCK(&peers);
build_mapping(v->name, v->value);
+ AST_LIST_UNLOCK(&peers);
v = v->next;
}
prune_mappings();
+
mark_peers();
cat = ast_category_browse(cfg, NULL);
while(cat) {
@@ -4948,6 +4976,7 @@
cat = ast_category_browse(cfg, cat);
}
prune_peers();
+
ast_config_destroy(cfg);
load_password();
if (globalpcmodel & DUNDI_MODEL_OUTBOUND)
@@ -4980,15 +5009,24 @@
pthread_join(previous_clearcachethreadid, NULL);
}
- close(netsocket);
- io_context_destroy(io);
-
mark_mappings();
prune_mappings();
mark_peers();
prune_peers();
- ast_sched_context_destroy(sched);
+ if (-1 < netsocket) {
+ close(netsocket);
+ netsocket = -1;
+ }
+ if (io) {
+ io_context_destroy(io);
+ io = NULL;
+ }
+
+ if (sched) {
+ ast_sched_context_destroy(sched);
+ sched = NULL;
+ }
return 0;
}
--
To view, visit https://gerrit.asterisk.org/9951
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib846b2b60d027f3a2c2b3b563d9a83a357dce1d6
Gerrit-Change-Number: 9951
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180821/a6e6834a/attachment-0001.html>
More information about the asterisk-code-review
mailing list