From 195cf61776bbf7ad1c739eca76d1de22fee37e15 Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Wed, 9 Feb 2005 15:14:44 +0000 Subject: [PATCH] In revision 1.29 timeout() was converted to ng_callout(). The difference is that the callout function installed via the ng_callout() method is guaranteed to NOT fire after the shutdown method was run (when a node is marked NGF_INVALID). Also, the shutdown method and the callout function are guaranteed to NOT run at the same time, as both require the writer lock. Thus we can safely ignore a zero return value from ng_uncallout() (callout_stop()) in shutdown methods, and go on with freeing the node. The said revision broke the node shutdown -- ng_bridge_timeout() is no longer fired after ng_bridge_shutdown() was run, resulting in a memory leak, dead nodes, and inability to unload the module. Fix this by cancelling the callout on shutdown, and moving part responsible for freeing a node resources from ng_bridge_timer() to ng_bridge_shutdown(). Noticed by: ru Submitted by: glebius, ru --- sys/netgraph/ng_bridge.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/sys/netgraph/ng_bridge.c b/sys/netgraph/ng_bridge.c index ae9ff6dc3156..bc36172415a4 100644 --- a/sys/netgraph/ng_bridge.c +++ b/sys/netgraph/ng_bridge.c @@ -754,16 +754,20 @@ ng_bridge_shutdown(node_p node) const priv_p priv = NG_NODE_PRIVATE(node); /* - * Shut down everything except the timer. There's no way to - * avoid another possible timeout event (it may have already - * been dequeued), so we can't free the node yet. + * Shut down everything including the timer. Even if the + * callout has already been dequeued and is about to be + * run, ng_bridge_timeout() won't be fired as the node + * is already marked NGF_INVALID, so we're safe to free + * the node now. */ KASSERT(priv->numLinks == 0 && priv->numHosts == 0, ("%s: numLinks=%d numHosts=%d", __func__, priv->numLinks, priv->numHosts)); + ng_uncallout(&priv->timer, node); + NG_NODE_SET_PRIVATE(node, NULL); + NG_NODE_UNREF(node); FREE(priv->tab, M_NETGRAPH_BRIDGE); - - /* NGF_INVALID flag is now set so node will be freed at next timeout */ + FREE(priv, M_NETGRAPH_BRIDGE); return (0); } @@ -956,8 +960,6 @@ ng_bridge_remove_hosts(priv_p priv, int linkNum) * we decrement link->loopCount for those links being muted due to * a detected loopback condition, and we remove any hosts from * the hashtable whom we haven't heard from in a long while. - * - * If the node has the NGF_INVALID flag set, our job is to kill it. */ static void ng_bridge_timeout(node_p node, hook_p hook, void *arg1, int arg2) @@ -967,14 +969,6 @@ ng_bridge_timeout(node_p node, hook_p hook, void *arg1, int arg2) int counter = 0; int linkNum; - /* If node was shut down, this is the final lingering timeout */ - if (NG_NODE_NOT_VALID(node)) { - FREE(priv, M_NETGRAPH_BRIDGE); - NG_NODE_SET_PRIVATE(node, NULL); - NG_NODE_UNREF(node); - return; - } - /* Update host time counters and remove stale entries */ for (bucket = 0; bucket < priv->numBuckets; bucket++) { struct ng_bridge_hent **hptr = &SLIST_FIRST(&priv->tab[bucket]);