Merge pull request #13 from jagerman/fix-reply-segfault

Fix python binding segfault

.drone.jsonnet

@@ -16,6 +16,7 @@ local apt_get_quiet = 'apt-get -o=Dpkg::Use-Pty=0 -q';
 local debian_pipeline(name,
                       image,
                       arch='amd64',
+                      distro='$$(lsb_release -sc)',
                       deps=default_deps,
                       extra_cmds=[],
                       jobs=6,
@@ -41,9 +42,9 @@ local debian_pipeline(name,
                   if loki_repo then [
                     'eatmydata ' + apt_get_quiet + ' install --no-install-recommends -y lsb-release',
                     'cp contrib/deb.oxen.io.gpg /etc/apt/trusted.gpg.d',
-                    'echo deb http://deb.oxen.io $$(lsb_release -sc) main >/etc/apt/sources.list.d/oxen.list',
-                    'echo deb http://deb.oxen.io/beta $$(lsb_release -sc) main >>/etc/apt/sources.list.d/oxen.list',
-                    'echo deb http://deb.oxen.io/staging $$(lsb_release -sc) main >>/etc/apt/sources.list.d/oxen.list',
+                    'echo deb http://deb.oxen.io ' + distro + ' main >/etc/apt/sources.list.d/oxen.list',
+                    'echo deb http://deb.oxen.io/beta ' + distro + ' main >>/etc/apt/sources.list.d/oxen.list',
+                    'echo deb http://deb.oxen.io/staging ' + distro + ' main >>/etc/apt/sources.list.d/oxen.list',
                     'eatmydata ' + apt_get_quiet + ' update',
                   ] else []
                 ) + [
@@ -92,14 +93,14 @@ local mac_builder(name,
 
 [
   // Various debian builds
-  debian_pipeline('Debian sid (amd64)', docker_base + 'debian-sid'),
+  debian_pipeline('Debian sid (amd64)', docker_base + 'debian-sid', distro='sid'),
   debian_pipeline('Debian stable (i386)', docker_base + 'debian-stable/i386'),
   debian_pipeline('Debian buster (amd64)', docker_base + 'debian-buster'),
   debian_pipeline('Ubuntu latest (amd64)', docker_base + 'ubuntu-rolling'),
   debian_pipeline('Ubuntu LTS (amd64)', docker_base + 'ubuntu-lts'),
 
   // ARM builds (ARM64 and armhf)
-  debian_pipeline('Debian sid (ARM64)', docker_base + 'debian-sid', arch='arm64', jobs=4),
+  debian_pipeline('Debian sid (ARM64)', docker_base + 'debian-sid', arch='arm64', jobs=4, distro='sid'),
   debian_pipeline('Debian stable (armhf)', docker_base + 'debian-stable/arm32v7', arch='arm64', jobs=4),
 
   // Macos builds:

setup.py

@@ -3,7 +3,7 @@ from setuptools import setup
 # Available at setup time due to pyproject.toml
 from pybind11.setup_helpers import Pybind11Extension, build_ext
 
-__version__ = "1.0.4"
+__version__ = "1.0.5"
 
 # Note:
 #   Sort input source files if you glob sources to ensure bit-for-bit

src/oxenmq.cpp

@@ -757,12 +757,12 @@ the background).)")
             }
 
             bool request = kwargs.contains("request") && kwargs["request"].cast<bool>();
-            std::optional<py::function> on_reply, on_reply_failure;
+            std::unique_ptr<py::function> on_reply, on_reply_failure;
             if (request) {
                 if (kwargs.contains("on_reply"))
-                    on_reply = kwargs["on_reply"].cast<py::function>();
+                    on_reply = std::make_unique<py::function>(kwargs["on_reply"].cast<py::function>());
                 if (kwargs.contains("on_reply_failure"))
-                    on_reply_failure = kwargs["on_reply_failure"].cast<py::function>();
+                    on_reply_failure = std::make_unique<py::function>(kwargs["on_reply_failure"].cast<py::function>());
             } else if (kwargs.contains("on_reply") || kwargs.contains("on_reply_failure")) {
                 throw std::logic_error{"Error: send(...) on_reply=/on_reply_failure= option "
                     "requires request=True (perhaps you meant to use `.request(...)` instead?)"};
@@ -795,29 +795,32 @@ the background).)")
                         hint, optional, incoming, outgoing, keep_alive, request_timeout,
                         std::move(qfail), std::move(qfull));
             } else {
-                auto reply_cb = [on_reply = std::move(on_reply), on_fail = std::move(on_reply_failure)]
-                    (bool success, std::vector<std::string> data) mutable {
+                auto reply_cb = [reply_rawptr = on_reply.release(), fail_rawptr = on_reply_failure.release()]
+                    (bool success, std::vector<std::string> data) {
                         // The gil here makes things tricky: the function invocation itself is
                         // already gil protected, but the *destruction* of the lambda isn't, and
                         // that breaks things because the destruction frees a python reference to
                         // the callback.  However oxenmq invokes this callback exactly once so we
-                        // can deal with it by stealing the captures out of the lambda to force
-                        // destruction here, with the gil held.
+                        // can deal with it by leaking raw pointers into the lambda captures then
+                        // reclaiming them the one and only time we are called.
                         py::gil_scoped_acquire gil;
-                        auto reply = std::move(on_reply);
-                        auto fail = std::move(on_fail);
+                        std::unique_ptr<py::function> reply{reply_rawptr};
+                        std::unique_ptr<py::function> fail{fail_rawptr};
 
-                        if (success ? !reply : !fail)
-                            return;
-                        py::list l;
                         if (success) {
-                            for (const auto& part : data)
-                                l.append(py::memoryview::from_memory(part.data(), part.size()));
-                            (*reply)(l);
-                        } else if (on_fail) {
-                            for (const auto& part : data)
-                                l.append(py::bytes(part.data(), part.size()));
-                            (*fail)(l);
+                            if (reply) {
+                                py::list l;
+                                for (const auto& part : data)
+                                    l.append(py::memoryview::from_memory(part.data(), part.size()));
+                                (*reply)(l);
+                            }
+                        } else {
+                            if (fail) {
+                                py::list l;
+                                for (const auto& part : data)
+                                    l.append(py::bytes(part.data(), part.size()));
+                                (*fail)(l);
+                            }
                         }
                     };