undo the python undo

fixup dbgsym?

fixup dbgsym?

fixup dbgsym?

fixup dbgsym?

fixup dbgsym?

fixup dbgsym?

fixup dbgsym?

fixup dbgsym

fixup dbgpkg

Revert "DISPATCH-1942 Use findPython when on CMake3.15+"

This reverts commit f486435

fixup too many decrefs

fixup travis yml; this is the best way ;DDDD

fixup travis yml?

fixup try -fno-omit-frame-pointer

fixup try -fno-omit-frame-pointer

fixup try -fno-omit-frame-pointer

fixup try -fno-omit-frame-pointer

fixup try -fno-omit-frame-pointer

try -fno-omit-frame-pointer

WIP: attempt at improving situation around DISPATCH-1962

WIP: attempt at improving situation around DISPATCH-1962

Author: jiridanek

.github/workflows/build.yaml

@@ -48,7 +48,7 @@ jobs:
       DispatchCMakeExtraArgs: >
         -DCMAKE_C_COMPILER_LAUNCHER=ccache
         -DCMAKE_CXX_COMPILER_LAUNCHER=ccache
-        -DCMAKE_C_FLAGS=-DQD_MEMORY_DEBUG
+        -DCMAKE_C_FLAGS="-DQD_MEMORY_DEBUG"
         -DQD_ENABLE_ASSERTIONS=ON
         -DCONSOLE_INSTALL=OFF
         -DUSE_BWRAP=ON
@@ -119,7 +119,7 @@ jobs:
             "-DCMAKE_INSTALL_PREFIX=${InstallPrefix}" \
             "-DCMAKE_BUILD_TYPE=${BuildType}" \
             "-GNinja" \
-            ${ProtonCMakeExtraArgs}
+            ${{env.ProtonCMakeExtraArgs}}
 
       - name: qpid-proton cmake build/install
         run: cmake --build "${ProtonBuildDir}" --config ${BuildType} -t install
@@ -134,7 +134,7 @@ jobs:
             "-DCMAKE_INSTALL_PREFIX=${InstallPrefix}" \
             "-DCMAKE_BUILD_TYPE=${BuildType}" \
             "-GNinja" \
-            ${DispatchCMakeExtraArgs}
+            ${{env.DispatchCMakeExtraArgs}}
 
       - name: qpid-dispatch cmake build/install
         run: cmake --build "${DispatchBuildDir}" --config ${BuildType} -t install

.travis.yml

@@ -50,6 +50,7 @@ jobs:
     env:
     - PATH="/usr/bin:$PATH" PROTON_VERSION=main BUILD_TYPE=Debug
     - DISPATCH_CMAKE_ARGS='-DRUNTIME_CHECK=asan'
+    - PYTHON_EXECUTABLE=/usr/bin/python2.7-dbg
   - name: "qdrouterd:Coverage"
     os: linux
     env:
@@ -59,6 +60,7 @@ jobs:
     env:
     - PATH="/usr/bin:$PATH" PROTON_VERSION=0.34.0 BUILD_TYPE=RelWithDebInfo
     - DISPATCH_CMAKE_ARGS='-DRUNTIME_CHECK=asan -DCMAKE_C_FLAGS=-DQD_MEMORY_DEBUG'
+    - PYTHON_EXECUTABLE=/usr/bin/python2.7-dbg
   - name: "qdrouterd:RelWithDebInfo+MemoryDebug (clang on focal)"
     os: linux
     dist: focal
@@ -159,7 +161,7 @@ addons:
     - cmake
     - libsasl2-dev
     - libssl-dev
-    - python2.7
+    - python2.7-dbg
     - python2.7-dev
     - sasl2-bin
     - swig
@@ -186,15 +188,15 @@ install:
 # Build and install proton from source.
 - mkdir qpid-proton/build
 - pushd qpid-proton/build
-- cmake .. -DCMAKE_INSTALL_PREFIX=$PREFIX -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DBUILD_BINDINGS=python
+- cmake .. -DCMAKE_INSTALL_PREFIX=$PREFIX -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DPYTHON_LIBRARY=/usr/lib/x86_64-linux-gnu/libpython2.7_d.so -DBUILD_BINDINGS=python
 - cmake --build . --target install -- -j $NPROC
 - popd
 
 before_script:
 - source qpid-proton/build/config.sh
 - mkdir build
 - pushd build
-- cmake .. -DCMAKE_INSTALL_PREFIX=$PREFIX -DCMAKE_BUILD_TYPE=${BUILD_TYPE} ${DISPATCH_CMAKE_ARGS}
+- cmake .. -DCMAKE_INSTALL_PREFIX=$PREFIX -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DPYTHON_EXECUTABLE=${PYTHON_EXECUTABLE} -DPYTHON_LIBRARY=/usr/lib/x86_64-linux-gnu/libpython2.7_d.so ${DISPATCH_CMAKE_ARGS}
 - . config.sh
 - make -j $NPROC
 

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);  // TODO: why not incref here? actually probably needed, I get a leak in test 9 without this, strange...
     qd->agent = agent;
 }
 
@@ -368,12 +369,18 @@ void qd_dispatch_free(qd_dispatch_t *qd)
     qd_connection_manager_free(qd->connection_manager);
     qd_policy_free(qd->policy);
     Py_XDECREF((PyObject*) qd->agent);
+//    Py_XDECREF(qd_python_module());  // hack
+    PyGC_Collect();  // run destructors while we still have the router around
+//    int ret = PyRun_SimpleString("import objgraph; import gc; gc.collect(); from qpid_dispatch_internal.management import config");
+//    assert(ret == 0);
+
+//    qd_python_finalize();
+
     qd_router_free(qd->router);
     qd_container_free(qd->container);
     qd_server_free(qd->server);
     qd_log_finalize();
-    qd_alloc_finalize();
-    qd_python_finalize();
+    qd_alloc_finalize();  // python objects may use alloc pool objects during finalization? TODO: they do that now  // actually, no, too late to do real work now
     qd_dispatch_set_router_id(qd, NULL);
     qd_dispatch_set_router_area(qd, NULL);
 }

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");
@@ -880,7 +896,7 @@ static void qd_python_setup(void)
         qd_register_constant(m, "TREATMENT_ANYCAST_CLOSEST",  QD_TREATMENT_ANYCAST_CLOSEST);
         qd_register_constant(m, "TREATMENT_ANYCAST_BALANCED", QD_TREATMENT_ANYCAST_BALANCED);
 
-        Py_INCREF(m);
+//        Py_INCREF(m);  // PyImport_ImportModule does the increment already
         dispatch_module = m;
     }
 

src/router_core/modules/mobile_sync/mobile.c

@@ -936,11 +936,13 @@ static void qcm_mobile_sync_final_CT(void *module_context)
 {
     qdrm_mobile_sync_t *msync = (qdrm_mobile_sync_t*) module_context;
 
+    qdr_core_unsubscribe(msync->message_sub1);
+    qdr_core_unsubscribe(msync->message_sub2);
     qdrc_event_unsubscribe_CT(msync->core, msync->event_sub);
     qdr_core_timer_free_CT(msync->core, msync->timer);
 
     //
-    // Don't explicitly unsubscribe the addresses, these are already gone at module-final time.
+    // Don't explicitly unsubscribe the addresses, these are already gone at module-final time.  // TODO: appears not?
     //
 
     free(msync);

src/router_core/router_core.c

@@ -150,37 +150,8 @@ void qdr_core_free(qdr_core_t *core)
     core->router_id = 0;
     core->router_area = 0;
 
-    // discard any left over actions
-
-    qdr_action_list_t  action_list;
-    DEQ_MOVE(core->action_list, action_list);
-    DEQ_APPEND(action_list, core->action_list_background);
-    qdr_action_t *action = DEQ_HEAD(action_list);
-    while (action) {
-        DEQ_REMOVE_HEAD(action_list);
-        action->action_handler(core, action, true);
-        free_qdr_action_t(action);
-        action = DEQ_HEAD(action_list);
-    }
-
-    // Drain the general work lists
-    qdr_general_handler(core);
-
-    //
-    // Free the core resources
-    //
-    sys_thread_free(core->thread);
-    sys_cond_free(core->action_cond);
-    sys_mutex_free(core->action_lock);
-    sys_mutex_free(core->work_lock);
-    sys_mutex_free(core->id_lock);
-    qd_timer_free(core->work_timer);
-
-    for (int i = 0; i <= QD_TREATMENT_LINK_BALANCED; ++i) {
-        if (core->forwarders[i]) {
-            free(core->forwarders[i]);
-        }
-    }
+    // I have to first run qdrc_endpoint_do_cleanup_CT src/router_core/core_link_endpoint.c:241
+    //  and only then qcm_edge_router_final_CT src/router_core/modules/edge_router/module.c:59
 
     qdr_link_route_t *link_route = 0;
     while ( (link_route = DEQ_HEAD(core->link_routes))) {
@@ -303,11 +274,46 @@ void qdr_core_free(qdr_core_t *core)
     }
     assert(DEQ_SIZE(core->streaming_connections) == 0);
 
-    // at this point all the conn identifiers have been freed
-    qd_hash_free(core->conn_id_hash);
+    // this now adds unsubscribe actions, so have to do before the discard next
+    //  plus I need to run it before core mutexes are freed
 
     qdr_modules_finalize(core);
 
+    // discard any left over actions
+
+    qdr_action_list_t  action_list;
+    DEQ_MOVE(core->action_list, action_list);
+    DEQ_APPEND(action_list, core->action_list_background);
+    qdr_action_t *action = DEQ_HEAD(action_list);
+    while (action) {
+        DEQ_REMOVE_HEAD(action_list);
+        action->action_handler(core, action, true);
+        free_qdr_action_t(action);
+        action = DEQ_HEAD(action_list);
+    }
+
+    // Drain the general work lists
+    qdr_general_handler(core);
+
+    //
+    // Free the core resources
+    //
+    sys_thread_free(core->thread);
+    sys_cond_free(core->action_cond);
+    sys_mutex_free(core->action_lock);
+    sys_mutex_free(core->work_lock);
+    sys_mutex_free(core->id_lock);
+    qd_timer_free(core->work_timer);
+
+    for (int i = 0; i <= QD_TREATMENT_LINK_BALANCED; ++i) {
+        if (core->forwarders[i]) {
+            free(core->forwarders[i]);
+        }
+    }
+
+    // at this point all the conn identifiers have been freed
+    qd_hash_free(core->conn_id_hash);
+
     qdr_agent_free(core->mgmt_agent);
 
     if (core->routers_by_mask_bit)       free(core->routers_by_mask_bit);

src/router_node.c

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

@@ -307,11 +307,12 @@ static PyMethodDef RouterAdapter_methods[] = {
 
 static PyTypeObject RouterAdapterType = {
     PyVarObject_HEAD_INIT(NULL, 0)
-    .tp_name = "dispatch.RouterAdapter",  /* tp_name*/
-    .tp_basicsize = sizeof(RouterAdapter),     /* tp_basicsize*/
-    .tp_flags = Py_TPFLAGS_DEFAULT,        /* tp_flags*/
-    .tp_doc = "Dispatch Router Adapter", /* tp_doc */
-    .tp_methods = RouterAdapter_methods,     /* tp_methods */
+    .tp_name = "dispatch.RouterAdapter",
+    .tp_basicsize = sizeof(RouterAdapter),
+    .tp_flags = Py_TPFLAGS_DEFAULT,
+    .tp_doc = "Dispatch Router Adapter",
+    .tp_methods = RouterAdapter_methods,
+    .tp_new = PyType_GenericNew,
 };
 
 
@@ -392,7 +393,6 @@ qd_error_t qd_router_python_setup(qd_router_t *router)
         return QD_ERROR_NONE;
 
     PyObject *pDispatchModule = qd_python_module();
-    RouterAdapterType.tp_new = PyType_GenericNew;
     PyType_Ready(&RouterAdapterType);
     QD_ERROR_PY_RET();
 
@@ -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);
+//    qd_python_finalize();
 }
 
 

tests/lsan.supp

@@ -38,13 +38,21 @@ leak:sys_mutex
 
 # expected, not a bug:
 #
-leak:qdr_core_subscribe
+#leak:qdr_core_subscribe
 
 # Ubuntu 16.04 (Xenial)
 #
 leak:_ctypes_alloc_format_string
 leak:__strdup
 
+# to be triaged; system_tests_http
+leak:^callback_healthz$
+leak:^callback_metrics$
+
+# to be triaged; system_tests_http1_adaptor
+leak:^pn_condition$
+leak:^pn_raw_connection$
+
 ####
 #### Miscellaneous 3rd party libraries:
 ####