BIND 8.2.3 REL Memory Leak

[Mark.Andrews at nominum.com]

	The problem was:

 			tmpdp = dp;
			DRCNTDEC(tmpdp);
			tmpdp->d_next = NULL;
 			dp = dp->d_next;

	the NULL assignment was too early.

	The following patch reverses the last two statements and
	generally cleans up.

	I'd been looking for DRCNTDEC/DRCNTINC missmatches and not
	seeing the assignment.  Moving DRCNTDEC in the general cleanup
	made it obvious.

	Mark
	
Index: bin/named/ns_update.c
===================================================================
RCS file: /proj/cvs/isc/bind8/src/bin/named/ns_update.c,v
retrieving revision 8.91
diff -u -r8.91 ns_update.c
--- ns_update.c	2001/03/12 01:49:00	8.91
+++ ns_update.c	2001/04/20 03:37:19
@@ -1395,17 +1395,21 @@
 			next_rrecp = NEXT(rrecp, r_link);
 		else
 			next_rrecp = PREV(rrecp, r_link);
+		UNLINK(*updlist, rrecp, r_link);
 		if (rrecp->r_section != S_UPDATE) {
 			if (rrecp->r_dp) {
 				DRCNTDEC(rrecp->r_dp);
 				if (rrecp->r_dp->d_rcnt == 0)
 					db_freedata(rrecp->r_dp);
+				rrecp->r_dp = NULL;
 			}
+			INSIST(rrecp->r_deldp == NULL);
 			res_freeupdrec(rrecp);
 			continue;
 		}
 		dname = rrecp->r_dname;
 		dp = rrecp->r_dp;
+		rrecp->r_dp = NULL;
 		if ((dp->d_mark & D_MARK_ADDED) != 0) {
 			if (rcode == NOERROR) {
 				/*
@@ -1438,37 +1442,19 @@
 					 */
 				}
 			}
-			DRCNTDEC(dp);
-			if (dp->d_rcnt == 0)
-				db_freedata(dp);
-		} else {
-			/*
-			 * Databuf's matching this were deleted by this
-			 * update, or were never executed (because we bailed
-			 * out early).
-			 */
-			DRCNTDEC(dp);
-			if (dp->d_rcnt == 0)
-				db_freedata(dp);
 		}
+		DRCNTDEC(dp);
+		if (dp->d_rcnt == 0)
+			db_freedata(dp);
 
 		/* Process deleted databuf's. */
 		dp = rrecp->r_deldp;
+		rrecp->r_deldp = NULL;
 		while (dp != NULL) {
 			tmpdp = dp;
-			DRCNTDEC(tmpdp);
-			tmpdp->d_next = NULL;
 			dp = dp->d_next;
-			if (rcode == NOERROR) {
-				if (tmpdp->d_rcnt)
-					ns_debug(ns_log_update, 1,
-					   "free_rrecp: type = %d, rcnt = %d",
-						 p_type(tmpdp->d_type),
-						 tmpdp->d_rcnt);
-				else {
-					db_freedata(tmpdp);
-				}
-			} else {
+			tmpdp->d_next = NULL;
+			if (rcode != NOERROR) {
 				/* Add the databuf back. */
 				tmpdp->d_mark &= ~D_MARK_DELETED;
 				if (db_update(dname, tmpdp, tmpdp, NULL,
@@ -1481,13 +1467,13 @@
 				      "free_rrecp: added back databuf 0x%0x",
 						 tmpdp);
 				}
-				if (tmpdp->d_rcnt == 0)
-					db_freedata(tmpdp);
 			}
+			DRCNTDEC(tmpdp);
+			if (tmpdp->d_rcnt == 0)
+				db_freedata(tmpdp);
 		}
 		res_freeupdrec(rrecp);
 	}
-	INIT_LIST(*updlist);
 }
 
 enum req_action

> 
> 	
> I am seeing a memory leak in BIND 8.2.3 REL
> The leak happens when named is processing DDNS update packets.
> 
> When building with -DMEMCLUSTER_RECORD & -DRECORD_STRINGS
> I am seeing about two leaks per DDNS update.  Here is what I am seeing
> at the bottom of named.memstats after doing an add and delete of a name:
> 
> Active Memory:
> db_save.c:142 15daf4:32
> db_save.c:142 15dabc:32
> db_save.c:142 15da84:32
> db_save.c:142 151f14:36
> 
> Looking at named.run, I can trace these savedata() calls to line 1337 in 
> ns_update.c
> 
> For some reason these databuf's are getting lost and never freed.
> 
> I would expect them to get freed when free_rrecp() is called in res_update().
> They are not.  When add a call to free_rrecp() at the bottom of
> req_update_private() just before the return(Finish) the databuf's get freed 
> and
> there is no longer a memory leak. This does not seem like the proper fix.
> Can anyone have a better fix?
> 
> Attach is a context diff.
> 
> -Kevin
> 
> 
> Index: ns_update.c
> ===================================================================
> RCS file: /proj/cvs/isc/bind/src/bin/named/ns_update.c,v
> retrieving revision 8.91
> diff -c -r8.91 ns_update.c
> *** ns_update.c	2001/03/12 01:49:00	8.91
> --- ns_update.c	2001/04/19 17:46:06
> ***************
> *** 1373,1378 ****
> --- 1373,1380 ----
>    	/* Make a log of the update. */
>    	(void) printupdatelog(from, &curupd, hp, zp, old_serial);
> 
> + 	free_rrecp(&curupd, 0, from);
> +
>    	return (Finish);
>    }
>    
> 
--
Mark Andrews, Nominum Inc.
1 Seymour St., Dundas Valley, NSW 2117, Australia
PHONE: +61 2 9871 4742                 INTERNET: Mark.Andrews at nominum.com