fix memory leak on session break.

When librelp session breaks unexpected, a memory leak could happen in sendq.c
when relpSendqeConstruct was called before the session break happened.

- Also Adds a new valgrind test basic-sessionbreak-vg.sh to reproduce
  the problem. without the fix, the test will fail.

- Adjusted testbench plumbing to support session break valgrind test.

closes: #188

src/sendq.c

@@ -38,7 +38,6 @@
 #include "sendq.h"
 #include "dbllinklist.h"
 
-
 /* handling for the sendq entries  */
 
 /** Construct a RELP sendq instance
@@ -141,6 +140,31 @@ relpSendqDestruct(relpSendq_t **ppThis)
 }
 
 
+/* remove the sendbuffer at the top of the queue. The removed buffer is destructed.
+ * This must only be called if there is at least one send buffer inside the queue.
+ * rgerhards, 2008-03-17
+ */
+static relpRetVal
+relpSendqDelFirstBuf(relpSendq_t *pThis)
+{
+	relpSendqe_t *pEntry;
+
+	ENTER_RELPFUNC;
+	RELPOBJ_assert(pThis, Sendq);
+	RELPOBJ_assert(pThis->pRoot, Sendqe);
+
+	pthread_mutex_lock(&pThis->mut);
+	pEntry = pThis->pRoot;
+	DLL_Del(pEntry, pThis->pRoot, pThis->pLast);
+	pthread_mutex_unlock(&pThis->mut);
+
+	CHKRet(relpSendqeDestruct(&pEntry));
+
+finalize_it:
+	LEAVE_RELPFUNC;
+}
+
+
 /* add a sendbuffer to the queue
  * the send buffer object is handed over, caller must no longer access it after
  * it has been passed into here.
@@ -149,7 +173,7 @@ relpSendqDestruct(relpSendq_t **ppThis)
 relpRetVal
 relpSendqAddBuf(relpSendq_t *pThis, relpSendbuf_t *pBuf, relpTcp_t *pTcp)
 {
-	relpSendqe_t *pEntry;
+	relpSendqe_t *pEntry = NULL;
 
 	ENTER_RELPFUNC;
 	RELPOBJ_assert(pThis, Sendq);
@@ -172,31 +196,15 @@ relpSendqAddBuf(relpSendq_t *pThis, relpSendbuf_t *pBuf, relpTcp_t *pTcp)
 		iRet = RELP_RET_OK; /* this code is well ok! */
 
 finalize_it:
-	LEAVE_RELPFUNC;
-}
-
-
-/* remove the sendbuffer at the top of the queue. The removed buffer is destructed.
- * This must only be called if there is at least one send buffer inside the queue.
- * rgerhards, 2008-03-17
- */
-relpRetVal
-relpSendqDelFirstBuf(relpSendq_t *pThis)
-{
-	relpSendqe_t *pEntry;
-
-	ENTER_RELPFUNC;
-	RELPOBJ_assert(pThis, Sendq);
-	RELPOBJ_assert(pThis->pRoot, Sendqe);
-
-	pthread_mutex_lock(&pThis->mut);
-	pEntry = pThis->pRoot;
-	DLL_Del(pEntry, pThis->pRoot, pThis->pLast);
-	pthread_mutex_unlock(&pThis->mut);
-
-	CHKRet(relpSendqeDestruct(&pEntry));
+	if(iRet != RELP_RET_OK) {
+		/* Free memory if pEntry was initialized in relpSendqeConstruct using
+		*	relpSendqDelFirstBuf()
+		*/
+		if (pEntry != NULL) {
+			relpSendqDelFirstBuf(pThis);
+		}
+	}
 
-finalize_it:
 	LEAVE_RELPFUNC;
 }
 

tests/Makefile.am

@@ -13,7 +13,9 @@ chkseq_SOURCES=chkseq.c
 have_tlslib_SOURCES=have_tlslib.c
 
 VALGRIND_TESTS= \
-	duplicate-receiver-vg.sh
+	duplicate-receiver-vg.sh \
+	basic-sessionbreak-vg.sh
+
 
 # TLS tests that work both with gnutls and openssl
 TLS_TESTS=  \

tests/basic-sessionbreak-vg.sh

@@ -0,0 +1,49 @@
+#!/bin/bash
+. ${srcdir:=$(pwd)}/test-framework.sh
+if [ "$VALGRIND" == "NO" ] ; then
+   echo "valgrind tests are not permitted by environment config"
+   exit 77
+fi
+if [ $(uname) = "SunOS" ] ; then
+   echo "This test currently does not work on all flavors of Solaris."
+   exit 77
+fi
+if [ $(uname) = "FreeBSD" ] ; then
+   echo "This test currently does not work on FreeBSD."
+   exit 77	
+fi
+export NUMMESSAGES=100000
+export NUMLOOPS=2
+
+#export valgrind="valgrind --malloc-fill=ff --free-fill=fe --log-fd=1"
+export valgrind="valgrind --malloc-fill=ff --free-fill=fe --leak-check=full --log-fd=1 --error-exitcode=10 --gen-suppressions=all"
+
+startup_receiver_valgrind --no-exit-on-error -e error.out.log --outfile $OUTFILE
+
+echo 'Send Message(s)...'
+for i in $(seq 1 $NUMLOOPS); do 
+        # How many times tcpflood runs in each threads
+	libtool --mode=execute ./send --no-exit-on-error -t 127.0.0.1 -p $TESTPORT -m "testmessage" -n $NUMMESSAGES $OPT_VERBOSE &
+	send_pid=$!
+
+	echo "started send instance $i (PID $send_pid)"
+
+	# Give it time to actually connect
+	sleep 1;
+
+	kill -9 $send_pid # >/dev/null 2>&1;
+	echo "killed send instance $i (PID $send_pid)"
+done;
+
+stop_receiver
+
+if [ "$RECEIVE_EXIT" -eq "10" ]; then
+	cleanup
+	printf 'valgrind run FAILED with exceptions\n'
+	printf "%s %s FAIL\n" "$(date +%H:%M:%S)" "$0"
+	exit 1
+fi
+
+#	check_output "testmessage"
+
+terminate

tests/receive.c

@@ -96,7 +96,7 @@ watchdog_expired(LIBRELP_ATTR_UNUSED const int sig)
 void LIBRELP_ATTR_NORETURN
 do_signal(const int sig)
 {
-	fprintf(stderr, "send: UNEXPECTED SIGNAL %d%s- terminating\n", sig,
+	fprintf(stderr, "receive: UNEXPECTED SIGNAL %d%s- terminating\n", sig,
 		sig == SIGPIPE ? " [SIGPIPE]" : "");
 	fflush(stderr);
 	exit(100);
@@ -197,6 +197,7 @@ onAuthErr(LIBRELP_ATTR_UNUSED void *pUsr, char *authinfo,
 static void
 exit_hdlr(void)
 {
+	fprintf(stderr, "receive: EXIT HDLR\n");
 	if(userdata != NULL) {
 		free(userdata->progname);
 		free(userdata);
@@ -233,6 +234,7 @@ int main(int argc, char *argv[]) {
 	int ret = 0;
 	int append_outfile = 0;
 	int watchdog_timeout = 60; /* one seconds looks like a good default */
+	int no_exit_on_err = 0;
 	const char *tlslib = NULL;
 	const char* outfile_name = NULL;
 
@@ -250,6 +252,7 @@ int main(int argc, char *argv[]) {
 		{"tls-lib", required_argument, 0, 'l'},
 		{"tlsconfcmd", required_argument, 0, 'c'},
 		{"watchdog-timeout", required_argument, 0, 'W'},
+		{"no-exit-on-error", no_argument, 0, 'N'},
 		{0, 0, 0, 0}
 	};
 
@@ -332,6 +335,9 @@ int main(int argc, char *argv[]) {
 		case 'z':
 			myPrivKeyFile = optarg;
 			break;
+		case 'N':
+			no_exit_on_err = 1;
+			break;
 		default:
 			print_usage();
 			return -1;
@@ -379,10 +385,14 @@ int main(int argc, char *argv[]) {
 		}
 	}
 
-	hdlr_enable(SIGTERM, terminate);
+	if (no_exit_on_err == 0) {
+		hdlr_enable(SIGPIPE, do_signal);
+	} else {
+		signal(SIGPIPE, SIG_IGN);
+	}
 	hdlr_enable(SIGUSR1, do_exit);
+	hdlr_enable(SIGTERM, terminate);
 	hdlr_enable(SIGALRM, watchdog_expired);
-	hdlr_enable(SIGPIPE, do_signal);
 
 	if(outfile_name != NULL) {
 		if((outFile = fopen(outfile_name, append_outfile ? "a" : "w")) == NULL) {

tests/send.c

@@ -242,6 +242,7 @@ int main(int argc, char *argv[]) {
 	int verbose = 0;
 	int protFamily = 2; /* IPv4=2, IPv6=10 */
 	int bEnableTLS = 0;
+	int no_exit_on_err = 0;
 	char *caCertFile = NULL;
 	char *myCertFile = NULL;
 	char *myPrivKeyFile = NULL;
@@ -276,6 +277,7 @@ int main(int argc, char *argv[]) {
 		{"kill-signal", required_argument, 0, KILL_SIGNAL},
 		{"kill-pid", required_argument, 0, KILL_PID},
 		{"connect-retries", required_argument, 0, CONNECT_RETRIES},
+		{"no-exit-on-error", no_argument, 0, 'N'},
 		{0, 0, 0, 0}
 	};
 
@@ -345,6 +347,9 @@ int main(int argc, char *argv[]) {
 		case 'z':
 			myPrivKeyFile = optarg;
 			break;
+		case 'N':
+			no_exit_on_err = 1;
+			break;
 		case KILL_ON_MSG:
 			kill_on_msg = atoi(optarg);
 			break;
@@ -364,7 +369,11 @@ int main(int argc, char *argv[]) {
 	}
 
 	atexit(exit_hdlr);
-	hdlr_enable(SIGPIPE, do_signal);
+	if (no_exit_on_err == 0) {
+		hdlr_enable(SIGPIPE, do_signal);
+	} else {
+		signal(SIGPIPE, SIG_IGN);
+	}
 
 	if(msgDataLen != 0 && msgDataLen < lenMsg) {
 		fprintf(stderr, "send: message is larger than configured message size!\n");

tests/test-framework.sh

@@ -88,6 +88,7 @@ stop_receiver() {
 	fi
 	kill $RECEIVE_PID &> /dev/null
 	wait $RECEIVE_PID
+	export RECEIVE_EXIT=$?
 	printf 'receiver %d stopped\n' $RECEIVE_PID
 }