Steve Singer ssinger at ca.afilias.info
Fri Mar 12 07:12:15 PST 2010
Chris Browne wrote:

I don't think this patch is right.

It looks to me like

* dstring_init calls  malloc and stores the result on the ->data member.
* dstring_free calls free on the ->data member.
* dstring_reset does not call free or malloc

So


> Update of /home/cvsd/slony1/slony1-engine/src/slon
> In directory main.slony.info:/tmp/cvs-serv11819
> 
> Modified Files:
>       Tag: REL_1_2_STABLE
> 	remote_listen.c remote_worker.c 
> Log Message:
> Apparently fixes to memory leaks via freeing values at more apropos times
> 
> 
> Index: remote_worker.c
> ===================================================================
<snip>
> 
> Index: remote_listen.c
> ===================================================================
> RCS file: /home/cvsd/slony1/slony1-engine/src/slon/remote_listen.c,v
> retrieving revision 1.31.2.5
> retrieving revision 1.31.2.6
> diff -C 2 -d -r1.31.2.5 -r1.31.2.6
> *** remote_listen.c	22 Dec 2009 17:10:10 -0000	1.31.2.5
> --- remote_listen.c	11 Mar 2010 21:38:04 -0000	1.31.2.6
> ***************
> *** 267,270 ****
> --- 267,271 ----
>   					 node->no_id,
>   					 dstring_data(&query1), PQresultErrorMessage(res));
> + 				dstring_free(&query1);
>   				PQclear(res);
>   				slon_disconnectdb(conn);

The above code is in a while loop.  dstring_init is called before the 
while loop, but each iteration of the while loop will try to put new 
queries into query1.   Once dstring_free is called on query1 I don't 
think it can be used anymore (dstring_init appears to be the only 
dstring macro that does a malloc ).

I think you only want to call dstring_free() following the end of the 
while() loop.


> ***************
> *** 342,345 ****
> --- 343,347 ----
>   		}
>   		if (oldpstate != poll_state) { /* Switched states... */
> + 			dstring_init(&query1);
>   			switch (poll_state) {
>   			case SLON_POLLSTATE_POLL:
> ***************
> *** 368,371 ****
> --- 370,374 ----
>   					 node->no_id,
>   					 dstring_data(&query1), PQresultErrorMessage(res));
> + 				dstring_free(&query1);
>   				PQclear(res);
>   				slon_disconnectdb(conn);
> ***************
> *** 375,378 ****
> --- 378,382 ----
>   				continue;
>   			}
> + 			dstring_free(&query1);
>   			PQclear(res);
>   		}
> 
> _______________________________________________
> Slony1-commit mailing list
> Slony1-commit at lists.slony.info
> http://lists.slony.info/mailman/listinfo/slony1-commit


-- 
Steve Singer
Afilias Canada
Data Services Developer
416-673-1142


More information about the Slony1-hackers mailing list