Skip to content
Browse files

fix memory leak in HTTPClient test

* add one more valgrind suppression in linux.supp
* conditional compile HTTPClient::failWhileFlushOtherWaiting which
  causes valgrind crash
* fix shared_ptr reference cycle in
  HTTPClient::failWhileRequestingOtherWaiting

Change-Id: I8980ef514fcb3339ac12e102f035e5a2235cf31f
Signed-off-by: Kevin Cai <kevinc@mozy.com>
Reviewed-on: https://gerrit.dechocorp.com/19894
Reviewed-by: Jeremy Stanley <jeremy@mozy.com>
Reviewed-by: Jenkins <hudsonbuild@mozy.com>
  • Loading branch information...
1 parent 297a0f0 commit 9adba5ea9b1bec8ca47a48e96a99a9757e9cccce @kevincai kevincai committed Jun 1, 2011
Showing with 68 additions and 2 deletions.
  1. +1 −1 Makefile.am
  2. +4 −1 buildtools/build.sh
  3. +1 −0 configure.ac
  4. +6 −0 linux.supp
  5. +49 −0 m4/check_valgrind.m4
  6. +7 −0 mordor/tests/http_client.cpp
View
2 Makefile.am
@@ -1,6 +1,6 @@
ACLOCAL_AMFLAGS=-I m4
AUTOMAKE_OPTIONS=nostdinc subdir-objects
-AM_CPPFLAGS=$(OPENSSL_INCLUDES) $(BOOST_CPPFLAGS) $(POSTGRESQL_CFLAGS) -I$(top_srcdir)
+AM_CPPFLAGS=$(OPENSSL_INCLUDES) $(BOOST_CPPFLAGS) $(POSTGRESQL_CFLAGS) $(VALGRIND_CPPFLAGS) -I$(top_srcdir)
AM_CXXFLAGS=-Wall -Werror -fno-strict-aliasing
nobase_include_HEADERS= \
View
5 buildtools/build.sh
@@ -8,11 +8,14 @@ if [ ! -f Makefile ]; then
export CXXFLAGS='-g -O0'
elif [ "$1" = "coverage" ]; then
export CXXFLAGS='-g -O0 -fprofile-arcs -ftest-coverage'
+ elif [ "$1" = "valgrind" ]; then
+ export CXXFLAGS='-g -O0'
+ VALGRINDFLAGS=--enable-valgrind=yes
else
ASSERTFLAGS=--disable-assert
fi
which pg_config >/dev/null || POSTGRESFLAGS=--without-postgresql
- ./configure --disable-shared $ASSERTFLAGS $POSTGRESFLAGS
+ ./configure --disable-shared $ASSERTFLAGS $POSTGRESFLAGS $VALGRINDFLAGS
fi
if [ $(uname) = 'Linux' ]; then
j=$(grep processor /proc/cpuinfo | wc -l)
View
1 configure.ac
@@ -27,6 +27,7 @@ AC_CHECK_PROG([PROTOC], [protoc], [protoc])
AC_SEARCH_LIBS([pthread_create], [pthread])
AC_SEARCH_LIBS([clock_gettime], [rt])
AC_SEARCH_LIBS([backtrace], [execinfo])
+AC_CHECK_VALGRIND
AX_CHECK_OPENSSL
AX_CHECK_ZLIB
AX_BOOST_BASE([1.40],, [AC_ERROR([Missing boost headers (1.40+)])])
View
6 linux.supp
@@ -67,3 +67,9 @@
fun:_ZN5boost9date_time16date_input_facetINS_9gregorian4dateEcSt19istreambuf_iteratorIcSt11char_traitsIcEEEC2ERKSsm
fun:_ZN5boost9date_time16time_input_facetINS_10posix_time5ptimeEcSt19istreambuf_iteratorIcSt11char_traitsIcEEEC1ERKSsm
}
+{
+ strftime
+ Memcheck:Cond
+ fun:__strftime_internal
+ fun:strftime_l
+}
View
49 m4/check_valgrind.m4
@@ -0,0 +1,49 @@
+# SYNOPSIS
+#
+# AC_CHECK_VALGRIND
+#
+# DESCRIPTION
+#
+# Test for enable/disable HAVE_VALGRIND macro
+#
+# This macro enable following option
+#
+# --disable-valgrind
+# --enable-valgrind=<yes|no>
+#
+# This macro calls:
+# AC_SUBST(VALGRIND_CPPFLAGS)
+#
+# And sets:
+#
+# HAVE_VALGRIND
+
+AC_DEFUN([AC_CHECK_VALGRIND],
+[
+ AC_ARG_ENABLE([valgrind],
+ AS_HELP_STRING(
+ [--with-valgrind@<:@=ARG@:>@],
+ [Compile with HAVE_VALGRIND macro defined (ARG=yes),
+ or disable it (ARG=no)
+ @<:@ARG=no@:>@ ]
+ ),
+ [
+ if test "$enableval" = "yes"; then
+ want_valgrind="yes"
+ else
+ want_valgrind="no"
+ fi
+ ],
+ [want_valgrind="no"]
+ )
+
+ AC_MSG_CHECKING([whether enable HAVE_VALGRIND macro])
+ if test x${want_valgrind} = "xyes" ; then
+ AC_MSG_RESULT([yes])
+ AC_DEFINE(HAVE_VALGRIND,1,[define if compiling for running valgrind ])
+ VALGRIND_CPPFLAGS="-DHAVE_VALGRIND"
+ AC_SUBST(VALGRIND_CPPFLAGS)
+ else
+ AC_MSG_RESULT([no])
+ fi
+])
View
7 mordor/tests/http_client.cpp
@@ -2262,6 +2262,8 @@ static void scheduleRequestAndThrowDummyException(ClientConnection::ptr conn,
}
+#ifndef HAVE_VALGRIND
+// failWhileFlushOtherWaiting test case causes valgrind crash, conditionally disable it
MORDOR_UNITTEST(HTTPClient, failWhileFlushOtherWaiting)
{
WorkerPool pool;
@@ -2282,7 +2284,10 @@ MORDOR_UNITTEST(HTTPClient, failWhileFlushOtherWaiting)
MORDOR_TEST_ASSERT_EXCEPTION(request->doRequest(), DummyException);
Scheduler::yield();
MORDOR_ASSERT(excepted);
+ // reset onWrite callback to break the conn->testStream->conn reference cycle
+ testStream->onWrite(0, 0);
}
+#endif
MORDOR_UNITTEST(HTTPClient, failWhileRequestingOtherWaiting)
@@ -2306,6 +2311,8 @@ MORDOR_UNITTEST(HTTPClient, failWhileRequestingOtherWaiting)
MORDOR_TEST_ASSERT_EXCEPTION(request->doRequest(), DummyException);
Scheduler::yield();
MORDOR_ASSERT(excepted);
+ // reset onWrite callback to break the conn->testStream->conn reference cycle
+ testStream->onWrite(0, 0);
}
MORDOR_UNITTEST(HTTPClient, failWhileFlushNoRequestBodyResponseHeaders)

0 comments on commit 9adba5e

Please sign in to comment.
Something went wrong with that request. Please try again.