DISPATCH-1962 Fix leak of IoAdapter_init

Author: jiridanek

.travis.yml

@@ -72,6 +72,12 @@ jobs:
     compiler: clang
     before_install:
     - sudo apt-get install clang-11 llvm-11-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
     # https://github.com/pypa/virtualenv/issues/1740
     # https://github.com/pypa/virtualenv/issues/1873
     - python -m pip install --user --upgrade pip
@@ -121,10 +127,15 @@ jobs:
     os: linux
     dist: focal
     compiler: clang
-    python:
-      - "3.9"
     before_install:
       - sudo apt-get install clang-11 llvm-11-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/*"
       # https://github.com/pypa/virtualenv/issues/1740
@@ -153,6 +164,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
       # https://github.com/pypa/virtualenv/issues/1740
       # https://github.com/pypa/virtualenv/issues/1873
       - python -m pip install --user --upgrade pip

src/dispatch.c

@@ -341,6 +341,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

@@ -562,7 +562,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,
 };
 
 
@@ -716,10 +717,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);
 }
 
@@ -812,10 +827,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,
 };
 
 
@@ -831,8 +849,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

@@ -2157,12 +2157,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

@@ -444,6 +444,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();
@@ -456,7 +457,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

@@ -34,21 +34,21 @@
 from __future__ import unicode_literals
 
 import errno
-import sys
-import time
-
-import __main__
 import functools
 import os
 import random
 import re
 import shutil
 import socket
 import subprocess
+import sys
+import time
 from copy import copy
 from datetime import datetime
 from subprocess import PIPE, STDOUT
 
+import __main__
+
 try:
     import queue as Queue  # 3.x
 except ImportError:
@@ -202,10 +202,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
 
@@ -381,7 +384,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):
         """
@@ -506,7 +513,6 @@ def __init__(self, name=None, config=Config(), pyinclude=None, wait=True,
         elif env_home:
             args += ['-I', os.path.join(env_home, 'python')]
 
-        args = os.environ.get('QPID_DISPATCH_RUNNER', '').split() + args
         super(Qdrouterd, self).__init__(args, name=name, expect=expect)
         self._management = None
         self._wait_ready = False