DISPATCH-1962 Fix leak of IoAdapter_init

Author: jiridanek

.travis.yml

@@ -75,6 +75,12 @@ jobs:
     - sudo add-apt-repository 'deb http://apt.llvm.org/focal/ llvm-toolchain-focal-13 main' -y
     - sudo apt-get update -q
     - sudo apt-get install -y clang-13 llvm-13-dev
+    # Python 3.8.2 in Ubuntu LTS has ASan issues
+    - sudo add-apt-repository -y ppa:deadsnakes/ppa
+    - sudo apt-get update
+    - sudo apt-get install python3.9 python3.9-dev python3.9-distutils
+    - sudo update-alternatives --install /usr/bin/python python /usr/bin/python3.9 10
+    - sudo update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.9 10
     # Update pip, it may prevent issues later
     - sudo apt-get install -y python3-pip
     - python3 -m pip install --user --upgrade pip
@@ -124,6 +130,13 @@ jobs:
     compiler: clang
     before_install:
       - sudo apt-get install clang-12 llvm-12-dev
+      # Python 3.8.2 in Ubuntu LTS has ASan issues
+      - sudo apt install -y python3-pip
+      - sudo add-apt-repository -y ppa:deadsnakes/ppa
+      - sudo apt-get update
+      - sudo apt-get install python3.9 python3.9-dev python3.9-distutils
+      - sudo update-alternatives --install /usr/bin/python python /usr/bin/python3.9 10
+      - sudo update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.9 10
       # Install and use the latest Node.js LTS version
       - nvm install "lts/*"
       # Update pip, it may prevent issues later
@@ -152,6 +165,11 @@ jobs:
     os: linux
     dist: focal
     before_install:
+      - sudo add-apt-repository -y ppa:deadsnakes/ppa
+      - sudo apt-get update
+      - sudo apt-get install python3.9 python3.9-dev python3.9-distutils
+      - sudo update-alternatives --install /usr/bin/python python /usr/bin/python3.9 10
+      - sudo update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.9 10
       # Update pip, it may prevent issues later
       - sudo apt-get install -y python3-pip
       - python3 -m pip install --user --upgrade pip

src/dispatch.c

@@ -340,6 +340,7 @@ qd_error_t qd_dispatch_prepare(qd_dispatch_t *qd)
 void qd_dispatch_set_agent(qd_dispatch_t *qd, void *agent) {
     assert(agent);
     assert(!qd->agent);
+    Py_IncRef(agent);
     qd->agent = agent;
 }
 

src/python_embedded.c

@@ -565,7 +565,8 @@ static PyTypeObject LogAdapterType = {
     .tp_dealloc   = (destructor)LogAdapter_dealloc,
     .tp_flags     = Py_TPFLAGS_DEFAULT,
     .tp_methods   = LogAdapter_methods,
-    .tp_init      = (initproc)LogAdapter_init
+    .tp_init      = (initproc)LogAdapter_init,
+    .tp_new       = PyType_GenericNew,
 };
 
 
@@ -719,10 +720,24 @@ static int IoAdapter_init(IoAdapter *self, PyObject *args, PyObject *kwds)
     return 0;
 }
 
+// visit all members which may conceivably participate in reference cycles
+static int IoAdapter_traverse(IoAdapter* self, visitproc visit, void *arg)
+{
+    Py_VISIT(self->handler);
+    return 0;
+}
+
+static int IoAdapter_clear(IoAdapter* self)
+{
+    Py_CLEAR(self->handler);
+    return 0;
+}
+
 static void IoAdapter_dealloc(IoAdapter* self)
 {
     qdr_core_unsubscribe(self->sub);
-    Py_DECREF(self->handler);
+    PyObject_GC_UnTrack(self);
+    IoAdapter_clear(self);
     Py_TYPE(self)->tp_free((PyObject*)self);
 }
 
@@ -815,10 +830,13 @@ static PyTypeObject IoAdapterType = {
     .tp_name      = DISPATCH_MODULE ".IoAdapter",
     .tp_doc       = "Dispatch IO Adapter",
     .tp_basicsize = sizeof(IoAdapter),
+    .tp_traverse  = (traverseproc)IoAdapter_traverse,
+    .tp_clear     = (inquiry)IoAdapter_clear,
     .tp_dealloc   = (destructor)IoAdapter_dealloc,
-    .tp_flags     = Py_TPFLAGS_DEFAULT,
+    .tp_flags     = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
     .tp_methods   = IoAdapter_methods,
     .tp_init      = (initproc)IoAdapter_init,
+    .tp_new       = PyType_GenericNew,
 };
 
 
@@ -834,8 +852,6 @@ static void qd_register_constant(PyObject *module, const char *name, uint32_t va
 
 static void qd_python_setup(void)
 {
-    LogAdapterType.tp_new = PyType_GenericNew;
-    IoAdapterType.tp_new  = PyType_GenericNew;
     if ((PyType_Ready(&LogAdapterType) < 0) || (PyType_Ready(&IoAdapterType) < 0)) {
         qd_error_py();
         qd_log(log_source, QD_LOG_CRITICAL, "Unable to initialize Adapters");

src/router_node.c

@@ -2161,12 +2161,12 @@ void qd_router_free(qd_router_t *router)
 
     qd_container_set_default_node_type(router->qd, 0, 0, QD_DIST_BOTH);
 
+    qd_router_python_free(router);
     qdr_core_free(router->router_core);
     qd_tracemask_free(router->tracemask);
     qd_timer_free(router->timer);
     sys_mutex_free(router->lock);
     qd_router_configure_free(router);
-    qd_router_python_free(router);
 
     free(router);
     free(node_id);

src/router_pynode.c

@@ -443,6 +443,7 @@ qd_error_t qd_router_python_setup(qd_router_t *router)
     // Instantiate the router
     //
     pyRouter = PyObject_CallObject(pClass, pArgs);
+    Py_DECREF(pClass);
     Py_DECREF(pArgs);
     Py_DECREF(adapterType);
     QD_ERROR_PY_RET();
@@ -455,7 +456,12 @@ qd_error_t qd_router_python_setup(qd_router_t *router)
 }
 
 void qd_router_python_free(qd_router_t *router) {
-    // empty
+    Py_XDECREF(pyRouter);
+    Py_XDECREF(pyTick);
+    Py_XDECREF(pySetMobileSeq);
+    Py_XDECREF(pySetMyMobileSeq);
+    Py_XDECREF(pyLinkLost);
+    PyGC_Collect();
 }
 
 

tests/lsan.supp

@@ -2,9 +2,6 @@
 # found by AddressSanitizer (ASAN)
 #
 
-# to be triaged; pretty much all tests
-leak:^IoAdapter_init$
-
 # to be triaged; pretty much all tests
 leak:^load_server_config$
 

tests/system_test.py

@@ -39,6 +39,8 @@
 import shutil
 import socket
 import subprocess
+import sys
+import time
 from copy import copy
 from datetime import datetime
 from subprocess import PIPE, STDOUT
@@ -191,10 +193,13 @@ def port_available(port, protocol_family='IPv4'):
 def wait_port(port, protocol_family='IPv4', **retry_kwargs):
     """Wait up to timeout for port (on host) to be connectable.
     Takes same keyword arguments as retry to control the timeout"""
+
     def check(e):
         """Only retry on connection refused"""
-        if not isinstance(e, socket.error) or not e.errno == errno.ECONNREFUSED:
-            raise
+        # TODO(DISPATCH-1539): in Python 3.3+ it is sufficient to catch only OSError
+        if isinstance(e, (socket.error, IOError, OSError)) and e.errno == errno.ECONNREFUSED:
+            return
+        raise
 
     host = None
 
@@ -346,7 +351,11 @@ def __init__(self, name=None, listen_port=None, wait=True,
             self.args += self.cl_args
         super(Http2Server, self).__init__(self.args, name=name, expect=expect)
         if wait:
-            self.wait_ready()
+            try:
+                self.wait_ready()
+            except Exception:
+                self.teardown()
+                raise
 
     def wait_ready(self, **retry_kwargs):
         """