buf_sock memory leak fixed, merged ccommon change introducing sockio …

…metrics (twitter#128)

* Squashed 'deps/ccommon/' changes from bb298bc..ab0edc8

ab0edc8 add metrics to track buf_sock objects (twitter#138)
ae02038 add travis ci (copied from pelikan) (twitter#139)
964645a Merge pull request twitter#135 from paegun/fix_cmake_install
70710c2 fixed and re-added cmake install instructions, w/ following notes: include directory made proper relative; opened pattern match b/c include directory should only contain files meant for inclusion.
5b095bc Merge pull request twitter#126 from kevyang/kevyang/120
426d56a return NULL when cc_alloc/cc_realloc is called with size == 0
ad271d4 Merge pull request twitter#133 from kevyang/132
47dbdba suppress unused parameter warning in debug_log_flush
648d19e Merge pull request twitter#127 from kevyang/56
780941a Merge pull request twitter#130 from kevyang/129
b8af6c0 Merge pull request twitter#131 from kevyang/128
6ecc318 fix duplicate symbols in cc_signal
080c41d cc_array - stop doing arithmetic on void *
d526f7a add debug oriented memory management
a4fb927 Update bool member rules in style guide
05c6e1e explicitly make ccommon a C project to avoid checking for CXX related variables

git-subtree-dir: deps/ccommon
git-subtree-split: ab0edc8

* sockio metrics help solve the memory leak

README.cmake

@@ -18,4 +18,4 @@ cmake ..
 make -j
 make test
 
-# executables can be found at $(topdir)/_bin/*
+# executables can be found at $(builddir)/_bin/*

README.md

@@ -55,7 +55,7 @@ mkdir _build && cd _build
 cmake ..
 make -j
 ```
-The executables can be found under ``./_bin/``
+The executables can be found under ``_bin/`` (under build directory)
 
 To run all the tests, including those on `ccommon`, run:
 ```sh

deps/ccommon/.travis.yml

@@ -0,0 +1,95 @@
+sudo: required
+language: c
+
+# using anchor to import sources into linux builds
+addons:
+  apt: &apt
+    sources:
+      - ubuntu-toolchain-r-test
+      - llvm-toolchain-precise-3.6
+      - llvm-toolchain-precise-3.7
+      - llvm-toolchain-precise
+
+# travis currently does not support directly setting gcc/clang with versions
+# (e.g. gcc-4.8) as value for the compiler key. So we will have to manually
+# request these packages and use environment varibles to create the matrix.
+#
+# In the case of osx, use brew to install the paritcular versions, instead of
+# specifying with packages.
+matrix:
+  include:
+    # gcc 4.8 on linux
+    - env: C_COMPILER=gcc-4.8
+      addons:
+        apt:
+          <<: *apt
+          packages: gcc-4.8
+
+    # gcc 4.9 on linux
+    - env: C_COMPILER=gcc-4.9
+      addons:
+        apt:
+          <<: *apt
+          packages: gcc-4.9
+
+    # gcc 5 on linux
+    - env: C_COMPILER=gcc-5
+      addons:
+        apt:
+          <<: *apt
+          packages: gcc-5
+
+    # clang 3.6 on linux
+    - env: C_COMPILER=clang-3.6
+      addons:
+        apt:
+          <<: *apt
+          packages: clang-3.6
+
+    # clang 3.7 on linux
+    - env: C_COMPILER=clang-3.7
+      addons:
+        apt:
+          <<: *apt
+          packages: clang-3.7
+
+    ## gcc 4.8 on osx
+    #- os: osx
+    #  env: FORMULA=gcc48 COMPILER=gcc C_COMPILER=gcc-4.8
+    #
+    ## gcc 4.9 on osx
+    #- os: osx
+    #  env: FORMULA=gcc49 COMPILER=gcc C_COMPILER=gcc-4.9
+    #
+    ## gcc 5 on osx
+    #- os: osx
+    #  env: FORMULA=gcc5 COMPILER=gcc C_COMPILER=gcc-5
+
+    # clang 3.6 on osx
+    - os: osx
+      osx_image: xcode6.4
+      env: C_COMPILER=clang
+
+    # clang 3.7 on osx
+    - os: osx
+      osx_image: xcode7.1
+      env: C_COMPILER=clang
+
+
+before_install:
+  # for osx: 0. update brew; 1. install cmake if missing; 2. (gcc) unlink pre-installed gcc; 3. (gcc) install desired version of gcc
+  - 'if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew update; fi'
+  - 'if [[ "$TRAVIS_OS_NAME" == "osx" && -z "$(which cmake)" ]]; then brew install cmake; fi' # xcode 7.1 is missing cmake
+  - 'if [[ "$TRAVIS_OS_NAME" == "osx" && "$COMPILER" == "gcc" ]]; then brew unlink gcc || true; fi' # ignore unlink errors
+  - 'if [[ "$TRAVIS_OS_NAME" == "osx" && "$COMPILER" == "gcc" ]]; then brew unlink $FORMULA || true; fi' # ignore unlink errors
+  - 'if [[ "$TRAVIS_OS_NAME" == "osx" && "$COMPILER" == "gcc" ]]; then brew install $FORMULA; fi'
+  - export CC=$C_COMPILER
+  - wget https://github.com/libcheck/check/releases/download/0.11.0/check-0.11.0.tar.gz
+  - tar xvfz check-0.11.0.tar.gz
+  - cd check-0.11.0 && ./configure && make && sudo make install && cd ..
+
+script:
+  - mkdir _build && cd _build
+  - cmake ..
+  - make -j
+  - make check

deps/ccommon/CMakeLists.txt

@@ -37,8 +37,8 @@ endif()
 
 # version info
 set(${PROJECT_NAME}_VERSION_MAJOR 1)
-set(${PROJECT_NAME}_VERSION_MINOR 0)
-set(${PROJECT_NAME}_VERSION_PATCH 2)
+set(${PROJECT_NAME}_VERSION_MINOR 1)
+set(${PROJECT_NAME}_VERSION_PATCH 0)
 set(${PROJECT_NAME}_VERSION
    ${${PROJECT_NAME}_VERSION_MAJOR}.${${PROJECT_NAME}_VERSION_MINOR}.${${PROJECT_NAME}_VERSION_PATCH}
    )

deps/ccommon/include/stream/cc_sockio.h

@@ -48,6 +48,7 @@ extern "C" {
 #include <cc_stream.h>
 
 #include <cc_define.h>
+#include <cc_metric.h>
 
 #include <inttypes.h>
 #include <stdlib.h>
@@ -62,6 +63,21 @@ typedef struct {
     SOCKIO_OPTION(OPTION_DECLARE)
 } sockio_options_st;
 
+/*          name                type            description */
+#define SOCKIO_METRIC(ACTION)                                                   \
+    ACTION( buf_sock_create,    METRIC_COUNTER, "# buf sock created"           )\
+    ACTION( buf_sock_create_ex, METRIC_COUNTER, "# buf sock create exceptions" )\
+    ACTION( buf_sock_destroy,   METRIC_COUNTER, "# buf sock destroyed"         )\
+    ACTION( buf_sock_curr,      METRIC_GAUGE,   "# buf sock allocated"         )\
+    ACTION( buf_sock_borrow,    METRIC_COUNTER, "# buf sock borrowed"          )\
+    ACTION( buf_sock_borrow_ex, METRIC_COUNTER, "# buf sock borrow exceptions" )\
+    ACTION( buf_sock_return,    METRIC_COUNTER, "# buf sock returned"          )\
+    ACTION( buf_sock_active,    METRIC_GAUGE,   "# buf sock being borrowed"    )
+
+typedef struct {
+    SOCKIO_METRIC(METRIC_DECLARE)
+} sockio_metrics_st;
+
 struct buf_sock {
     /* these fields are useful for resource managmenet */
     STAILQ_ENTRY(buf_sock)  next;
@@ -79,7 +95,7 @@ struct buf_sock {
 
 STAILQ_HEAD(buf_sock_sqh, buf_sock); /* corresponding header type for the STAILQ */
 
-void sockio_setup(sockio_options_st *options);
+void sockio_setup(sockio_options_st *options, sockio_metrics_st *metrics);
 void sockio_teardown(void);
 
 struct buf_sock *buf_sock_create(void);     /* stream_get_fn */

deps/ccommon/src/CMakeLists.txt

@@ -35,3 +35,10 @@ set_target_properties (${PROJECT_NAME}-shared
     OUTPUT_NAME ${PROJECT_NAME}
     VERSION ${${PROJECT_NAME}_VERSION}
     SOVERSION 0)
+
+# install instructions
+install(TARGETS ${PROJECT_NAME}-static DESTINATION lib)
+install(TARGETS ${PROJECT_NAME}-shared DESTINATION lib)
+install(DIRECTORY ../include/
+    DESTINATION include/${PROJECT_NAME}-${${PROJECT_NAME}_RELEASE_VERSION}
+    FILES_MATCHING PATTERN "*")

deps/ccommon/src/stream/cc_sockio.c

@@ -42,7 +42,9 @@
 FREEPOOL(buf_sock_pool, buf_sockq, buf_sock);
 struct buf_sock_pool bsp;
 
+static bool sockio_init = false;
 static bool bsp_init = false;
+static sockio_metrics_st *sockio_metrics = NULL;
 
 rstatus_i
 buf_tcp_read(struct buf_sock *s)
@@ -212,6 +214,7 @@ buf_sock_create(void)
 
     s = (struct buf_sock *)cc_alloc(sizeof(struct buf_sock));
     if (s == NULL) {
+        INCR(sockio_metrics, buf_sock_create_ex);
         return NULL;
     }
     STAILQ_NEXT(s, next) = NULL;
@@ -235,12 +238,16 @@ buf_sock_create(void)
         goto error;
     }
 
+    INCR(sockio_metrics, buf_sock_create);
+    INCR(sockio_metrics, buf_sock_curr);
+
     log_verb("created buffered socket %p", s);
 
     return s;
 
 error:
     log_info("buffered socket creation failed");
+    INCR(sockio_metrics, buf_sock_create_ex);
     buf_sock_destroy(&s);
 
     return NULL;
@@ -261,6 +268,8 @@ buf_sock_destroy(struct buf_sock **s)
     cc_free(*s);
 
     *s = NULL;
+    INCR(sockio_metrics, buf_sock_destroy);
+    DECR(sockio_metrics, buf_sock_curr);
 }
 
 static void
@@ -331,10 +340,13 @@ buf_sock_borrow(void)
     FREEPOOL_BORROW(s, &bsp, next, buf_sock_create);
     if (s == NULL) {
         log_debug("borrow buffered socket failed: OOM or over limit");
+        INCR(sockio_metrics, buf_sock_borrow_ex);
 
         return NULL;
     }
     buf_sock_reset(s);
+    INCR(sockio_metrics, buf_sock_borrow);
+    INCR(sockio_metrics, buf_sock_active);
 
     log_verb("borrowed buffered socket %p", s);
 
@@ -354,18 +366,29 @@ buf_sock_return(struct buf_sock **s)
     FREEPOOL_RETURN(*s, &bsp, next);
 
     *s = NULL;
+    INCR(sockio_metrics, buf_sock_return);
+    DECR(sockio_metrics, buf_sock_active);
 }
 
 void
-sockio_setup(sockio_options_st *options)
+sockio_setup(sockio_options_st *options, sockio_metrics_st *metrics)
 {
     uint32_t max = BUFSOCK_POOLSIZE;
 
+    log_info("set up the %s module", SOCKIO_MODULE_NAME);
+
+    if (sockio_init) {
+        log_warn("%s has already been setup, overwrite", SOCKIO_MODULE_NAME);
+    }
+
+    sockio_metrics = metrics;
+
     if (options != NULL) {
         max = option_uint(&options->buf_sock_poolsize);
     }
 
     buf_sock_pool_create(max);
+    sockio_init = true;
 }
 
 void

src/core/data/server.c

@@ -78,6 +78,7 @@ _tcp_accept(struct buf_sock *ss)
     }
 
     if (!ss->hdl->accept(sc, s->ch)) {
+        buf_sock_return(&s);
         return false;
     }
 

src/server/pingserver/main.c

@@ -103,7 +103,7 @@ setup(void)
     buf_setup(&setting.buf, &stats.buf);
     dbuf_setup(&setting.dbuf, &stats.dbuf);
     event_setup(&stats.event);
-    sockio_setup(&setting.sockio);
+    sockio_setup(&setting.sockio, &stats.sockio);
     tcp_setup(&setting.tcp, &stats.tcp);
     timing_wheel_setup(&stats.timing_wheel);
 

src/server/pingserver/stats.c

@@ -11,6 +11,7 @@ struct stats stats = {
     { DBUF_METRIC(METRIC_INIT)          },
     { EVENT_METRIC(METRIC_INIT)         },
     { LOG_METRIC(METRIC_INIT)           },
+    { SOCKIO_METRIC(METRIC_INIT)        },
     { TCP_METRIC(METRIC_INIT)           },
     { TIMING_WHEEL_METRIC(METRIC_INIT)  },
 };

src/server/pingserver/stats.h

@@ -11,6 +11,7 @@
 #include <cc_event.h>
 #include <cc_log.h>
 #include <channel/cc_tcp.h>
+#include <stream/cc_sockio.h>
 #include <time/cc_wheel.h>
 
 struct stats {
@@ -27,6 +28,7 @@ struct stats {
     dbuf_metrics_st             dbuf;
     event_metrics_st            event;
     log_metrics_st              log;
+    sockio_metrics_st           sockio;
     tcp_metrics_st              tcp;
     timing_wheel_metrics_st     timing_wheel;
 };

src/server/slimcache/main.c

@@ -108,7 +108,7 @@ setup(void)
     buf_setup(&setting.buf, &stats.buf);
     dbuf_setup(&setting.dbuf, &stats.dbuf);
     event_setup(&stats.event);
-    sockio_setup(&setting.sockio);
+    sockio_setup(&setting.sockio, &stats.sockio);
     tcp_setup(&setting.tcp, &stats.tcp);
     timing_wheel_setup(&stats.timing_wheel);
 

src/server/slimcache/stats.c

@@ -16,6 +16,7 @@ struct stats stats = {
     { DBUF_METRIC(METRIC_INIT)          },
     { EVENT_METRIC(METRIC_INIT)         },
     { LOG_METRIC(METRIC_INIT)           },
+    { SOCKIO_METRIC(METRIC_INIT)        },
     { TCP_METRIC(METRIC_INIT)           },
     { TIMING_WHEEL_METRIC(METRIC_INIT)  },
 };

src/server/slimcache/stats.h

@@ -11,6 +11,7 @@
 #include <cc_event.h>
 #include <cc_log.h>
 #include <channel/cc_tcp.h>
+#include <stream/cc_sockio.h>
 #include <time/cc_wheel.h>
 
 struct stats {
@@ -32,6 +33,7 @@ struct stats {
     dbuf_metrics_st             dbuf;
     event_metrics_st            event;
     log_metrics_st              log;
+    sockio_metrics_st           sockio;
     tcp_metrics_st              tcp;
     timing_wheel_metrics_st     timing_wheel;
 };

src/server/twemcache/main.c

@@ -103,7 +103,7 @@ setup(void)
     buf_setup(&setting.buf, &stats.buf);
     dbuf_setup(&setting.dbuf, &stats.dbuf);
     event_setup(&stats.event);
-    sockio_setup(&setting.sockio);
+    sockio_setup(&setting.sockio, &stats.sockio);
     tcp_setup(&setting.tcp, &stats.tcp);
     timing_wheel_setup(&stats.timing_wheel);
 

src/server/twemcache/stats.c

@@ -16,6 +16,7 @@ struct stats stats = {
     { DBUF_METRIC(METRIC_INIT)          },
     { EVENT_METRIC(METRIC_INIT)         },
     { LOG_METRIC(METRIC_INIT)           },
+    { SOCKIO_METRIC(METRIC_INIT)        },
     { TCP_METRIC(METRIC_INIT)           },
     { TIMING_WHEEL_METRIC(METRIC_INIT)  },
 };

src/server/twemcache/stats.h

@@ -12,6 +12,7 @@
 #include <cc_event.h>
 #include <cc_log.h>
 #include <channel/cc_tcp.h>
+#include <stream/cc_sockio.h>
 #include <time/cc_wheel.h>
 
 struct stats {
@@ -33,6 +34,7 @@ struct stats {
     dbuf_metrics_st             dbuf;
     event_metrics_st            event;
     log_metrics_st              log;
+    sockio_metrics_st           sockio;
     tcp_metrics_st              tcp;
     timing_wheel_metrics_st     timing_wheel;
 };