From 6389708701f7120ffa60582a7b6a043c78f1f11c Mon Sep 17 00:00:00 2001 From: John Emhoff Date: Mon, 23 Feb 2015 15:03:50 +0000 Subject: [PATCH 01/10] Fix destructor order for _MessageReader --- capnp/lib/capnp.pyx | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index 33f191a..47862c4 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -3229,8 +3229,7 @@ cdef class _MessageReader: """ cdef public object _parent cdef schema_cpp.MessageReader * thisptr - def __dealloc__(self): - del self.thisptr + def __init__(self): raise NotImplementedError("This is an abstract base class") @@ -3290,6 +3289,10 @@ cdef class _StreamFdMessageReader(_MessageReader): self.thisptr = new schema_cpp.StreamFdMessageReader(fd, opts) + def __dealloc__(self): + del self.thisptr + + cdef class _PackedMessageReader(_MessageReader): """Read a Cap'n Proto message from a file descriptor in a packed manner @@ -3318,6 +3321,10 @@ cdef class _PackedMessageReader(_MessageReader): self.thisptr = new schema_cpp.PackedMessageReader(stream, opts) return self + def __dealloc__(self): + del self.thisptr + + cdef class _PackedMessageReaderBytes(_MessageReader): cdef schema_cpp.ArrayInputStream * stream @@ -3340,6 +3347,7 @@ cdef class _PackedMessageReaderBytes(_MessageReader): self.thisptr = new schema_cpp.PackedMessageReader(deref(self.stream), opts) def __dealloc__(self): + del self.thisptr del self.stream cdef class _InputMessageReader(_MessageReader): @@ -3370,6 +3378,10 @@ cdef class _InputMessageReader(_MessageReader): self.thisptr = new schema_cpp.InputStreamMessageReader(stream, opts) return self + def __dealloc__(self): + del self.thisptr + + cdef class _PackedFdMessageReader(_MessageReader): """Read a Cap'n Proto message from a file descriptor in a packed manner @@ -3392,6 +3404,10 @@ cdef class _PackedFdMessageReader(_MessageReader): self.thisptr = new schema_cpp.PackedFdMessageReader(fd, opts) + def __dealloc__(self): + del self.thisptr + + cdef class _MultipleMessageReader: cdef schema_cpp.FdInputStream * stream cdef schema_cpp.BufferedInputStream * buffered_stream @@ -3574,6 +3590,10 @@ cdef class _FlatArrayMessageReader(_MessageReader): self.thisptr = new schema_cpp.FlatArrayMessageReader(schema_cpp.WordArrayPtr(ptr, sz//8)) + def __dealloc__(self): + del self.thisptr + + @cython.internal cdef class _FlatMessageBuilder(_MessageBuilder): cdef object _object_to_pin From b2daf3911dd0e749df3557eff426671840ade7d1 Mon Sep 17 00:00:00 2001 From: Jason Paryani Date: Mon, 23 Feb 2015 11:21:19 -0800 Subject: [PATCH 02/10] Default to not using cython --- README.md | 2 +- setup.py | 24 +++++++----------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 8cc915e..7575b7d 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ Install with `pip install pycapnp`. You can set the CC environment variable to c Or you can clone the repo like so: git clone https://github.com/jparyani/pycapnp.git - pip install ./pycapnp + pip install --install-option '--force-cython' ./pycapnp Note: for OSX, if using clang from Xcode 5, you will need to set `CFLAGS` like so: diff --git a/setup.py b/setup.py index 51a9fd3..9041997 100644 --- a/setup.py +++ b/setup.py @@ -1,15 +1,7 @@ #!/usr/bin/env python from __future__ import print_function -use_cython = True -try: - from Cython.Build import cythonize - import Cython -except ImportError: - use_cython = False - -if use_cython and Cython.__version__ < '0.19.1': - use_cython = False +use_cython = False import pkg_resources setuptools_version = pkg_resources.get_distribution("setuptools").version @@ -23,10 +15,6 @@ import sys from buildutils import test_build, fetch_libcapnp, build_libcapnp, info from distutils.errors import CompileError from distutils.extension import Extension -if use_cython: - from Cython.Distutils import build_ext as build_ext_c -else: - from distutils.command.build_ext import build_ext as build_ext_c _this_dir = os.path.dirname(__file__) @@ -85,15 +73,15 @@ if force_bundled_libcapnp: force_system_libcapnp = "--force-system-libcapnp" in sys.argv if force_system_libcapnp: sys.argv.remove("--force-system-libcapnp") -disable_cython = "--disable-cython" in sys.argv -if disable_cython: - sys.argv.remove("--disable-cython") - use_cython = False force_cython = "--force-cython" in sys.argv if force_cython: sys.argv.remove("--force-cython") use_cython = True +if use_cython: + from Cython.Distutils import build_ext as build_ext_c +else: + from distutils.command.build_ext import build_ext as build_ext_c class build_libcapnp_ext(build_ext_c): def build_extension(self, ext): @@ -127,6 +115,8 @@ class build_libcapnp_ext(build_ext_c): return build_ext_c.run(self) if use_cython: + from Cython.Build import cythonize + import Cython extensions = cythonize('capnp/lib/*.pyx') else: extensions = [Extension("capnp.lib.capnp", ["capnp/lib/capnp.cpp"], From 7dc6f28ed9c64c9b3388ccc941009118eaade5cb Mon Sep 17 00:00:00 2001 From: Jason Paryani Date: Mon, 23 Feb 2015 11:23:39 -0800 Subject: [PATCH 03/10] Bump version to v0.5.3 --- CHANGELOG.md | 5 +++++ setup.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9464ead..eb7690e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v0.5.2 (2015-02-23) +- Fix possible crash due to bad destructor ordering in MessageReader (by @JohnEmhoff) +- Default to no longer using cython + + ## v0.5.2 (2015-02-20) - Add read\_multiple\_bytes/read\_multiple\_bytes\_packed methods - Added Python 3.4 to the travis build matrix diff --git a/setup.py b/setup.py index 9041997..3a467c6 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,7 @@ _this_dir = os.path.dirname(__file__) MAJOR = 0 MINOR = 5 -MICRO = 2 +MICRO = 3 VERSION = '%d.%d.%d' % (MAJOR, MINOR, MICRO) From 6c5e363406cc9a471998579b578424939404bab9 Mon Sep 17 00:00:00 2001 From: Jason Paryani Date: Mon, 23 Feb 2015 17:35:29 -0800 Subject: [PATCH 04/10] Fix typo in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb7690e..81f87e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## v0.5.2 (2015-02-23) +## v0.5.3 (2015-02-23) - Fix possible crash due to bad destructor ordering in MessageReader (by @JohnEmhoff) - Default to no longer using cython From 6e94182a3ed2ad367b370dbb852ecf8142770b0b Mon Sep 17 00:00:00 2001 From: Jason Paryani Date: Tue, 24 Feb 2015 11:22:43 -0800 Subject: [PATCH 05/10] Add bootstrap RPC methods This is the new way for initializing RPC connections. It replaces the deprecated restore functionality --- capnp/helpers/helpers.pxd | 1 + capnp/helpers/rpcHelper.h | 7 +++++++ capnp/includes/capnp_cpp.pxd | 1 + capnp/lib/capnp.pxd | 2 +- capnp/lib/capnp.pyx | 28 ++++++++++++++++++++++------ test/test_rpc.py | 14 ++++++++++++++ 6 files changed, 46 insertions(+), 7 deletions(-) diff --git a/capnp/helpers/helpers.pxd b/capnp/helpers/helpers.pxd index d0b1edc..28acaa1 100644 --- a/capnp/helpers/helpers.pxd +++ b/capnp/helpers/helpers.pxd @@ -30,6 +30,7 @@ cdef extern from "capnp/helpers/rpcHelper.h": Capability.Client restoreHelper(RpcSystem&, MessageReader&) Capability.Client restoreHelper(RpcSystem&, AnyPointer.Reader&) Capability.Client restoreHelper(RpcSystem&, AnyPointer.Builder&) + Capability.Client bootstrapHelper(RpcSystem&) RpcSystem makeRpcClientWithRestorer(TwoPartyVatNetwork&, PyRestorer&) PyPromise connectServer(TaskSet &, PyRestorer &, AsyncIoContext *, StringPtr) diff --git a/capnp/helpers/rpcHelper.h b/capnp/helpers/rpcHelper.h index a7d54bd..5ae53e3 100644 --- a/capnp/helpers/rpcHelper.h +++ b/capnp/helpers/rpcHelper.h @@ -73,6 +73,13 @@ capnp::Capability::Client restoreHelper(capnp::RpcSystem& client) { + capnp::MallocMessageBuilder hostIdMessage(8); + auto hostId = hostIdMessage.initRoot(); + hostId.setSide(capnp::rpc::twoparty::Side::SERVER); + return client.bootstrap(hostId); +} + template capnp::RpcSystem makeRpcClientWithRestorer( diff --git a/capnp/includes/capnp_cpp.pxd b/capnp/includes/capnp_cpp.pxd index 4e8dd8f..c246d0b 100644 --- a/capnp/includes/capnp_cpp.pxd +++ b/capnp/includes/capnp_cpp.pxd @@ -345,6 +345,7 @@ cdef extern from "capnp/rpc-twoparty.h" namespace " ::capnp": VoidPromise onDisconnect() VoidPromise onDrained() RpcSystem makeRpcServer(TwoPartyVatNetwork&, PyRestorer&) + RpcSystem makeRpcServerBootstrap"makeRpcServer"(TwoPartyVatNetwork&, Capability.Client) RpcSystem makeRpcClient(TwoPartyVatNetwork&) cdef extern from "capnp/dynamic.h" namespace " ::capnp": diff --git a/capnp/lib/capnp.pxd b/capnp/lib/capnp.pxd index 1f5ddea..d7e3360 100644 --- a/capnp/lib/capnp.pxd +++ b/capnp/lib/capnp.pxd @@ -1,6 +1,6 @@ from capnp.includes cimport capnp_cpp as capnp from capnp.includes cimport schema_cpp -from capnp.includes.capnp_cpp cimport Schema as C_Schema, StructSchema as C_StructSchema, InterfaceSchema as C_InterfaceSchema, EnumSchema as C_EnumSchema, ListSchema as C_ListSchema, DynamicStruct as C_DynamicStruct, DynamicValue as C_DynamicValue, Type as C_Type, DynamicList as C_DynamicList, SchemaParser as C_SchemaParser, ParsedSchema as C_ParsedSchema, VOID, ArrayPtr, StringPtr, String, StringTree, DynamicOrphan as C_DynamicOrphan, AnyPointer as C_DynamicObject, DynamicCapability as C_DynamicCapability, Request, Response, RemotePromise, PyPromise, VoidPromise, CallContext, PyRestorer, RpcSystem, makeRpcServer, makeRpcClient, Capability as C_Capability, TwoPartyVatNetwork as C_TwoPartyVatNetwork, Side, AsyncIoStream, Own, makeTwoPartyVatNetwork, PromiseFulfillerPair as C_PromiseFulfillerPair, copyPromiseFulfillerPair, newPromiseAndFulfiller, PyArray, DynamicStruct_Builder +from capnp.includes.capnp_cpp cimport Schema as C_Schema, StructSchema as C_StructSchema, InterfaceSchema as C_InterfaceSchema, EnumSchema as C_EnumSchema, ListSchema as C_ListSchema, DynamicStruct as C_DynamicStruct, DynamicValue as C_DynamicValue, Type as C_Type, DynamicList as C_DynamicList, SchemaParser as C_SchemaParser, ParsedSchema as C_ParsedSchema, VOID, ArrayPtr, StringPtr, String, StringTree, DynamicOrphan as C_DynamicOrphan, AnyPointer as C_DynamicObject, DynamicCapability as C_DynamicCapability, Request, Response, RemotePromise, PyPromise, VoidPromise, CallContext, PyRestorer, RpcSystem, makeRpcServer, makeRpcServerBootstrap, makeRpcClient, Capability as C_Capability, TwoPartyVatNetwork as C_TwoPartyVatNetwork, Side, AsyncIoStream, Own, makeTwoPartyVatNetwork, PromiseFulfillerPair as C_PromiseFulfillerPair, copyPromiseFulfillerPair, newPromiseAndFulfiller, PyArray, DynamicStruct_Builder from capnp.includes.schema_cpp cimport Node as C_Node, EnumNode as C_EnumNode from capnp.includes.types cimport * from capnp.helpers.non_circular cimport reraise_kj_exception diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index 47862c4..9a7efd2 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -2253,6 +2253,9 @@ cdef class TwoPartyClient: return self.restore(ref) + cpdef bootstrap(self) except +reraise_kj_exception: + return _CapabilityClient()._init(helpers.bootstrapHelper(deref(self.thisptr)), self) + cpdef on_disconnect(self) except +reraise_kj_exception: return _VoidPromise()._init(deref(self._network.thisptr).onDisconnect()) @@ -2263,12 +2266,18 @@ cdef class TwoPartyServer: cdef public _Restorer _restorer cdef public _AsyncIoStream _stream cdef object _port - cdef public object port_promise + cdef public object port_promise, _bootstrap cdef capnp.TaskSet * _task_set cdef capnp.ErrorHandler _error_handler - def __init__(self, socket, restorer, server_socket=None): - self._restorer = _convert_restorer(restorer) + def __init__(self, socket, restorer=None, server_socket=None, bootstrap=None): + if not restorer and not bootstrap: + raise KjException("You must provide either a bootstrap interface or a restorer (deperecated) to a server constructor.") + + cdef _InterfaceSchema schema + self._restorer = None + self._bootstrap = None + if isinstance(socket, basestring): self._connect(socket) else: @@ -2277,11 +2286,19 @@ cdef class TwoPartyServer: self._server_socket = server_socket self._port = 0 self._network = _TwoPartyVatNetwork()._init(self._stream, capnp.SERVER) - self.thisptr = new RpcSystem(makeRpcServer(deref(self._network.thisptr), deref(self._restorer.thisptr))) + + if bootstrap: + self._bootstrap = bootstrap + schema = bootstrap.schema + self.thisptr = new RpcSystem(makeRpcServerBootstrap(deref(self._network.thisptr), helpers.server_to_client(schema.thisptr, bootstrap))) + elif restorer: + self._restorer = _convert_restorer(restorer) + self.thisptr = new RpcSystem(makeRpcServer(deref(self._network.thisptr), deref(self._restorer.thisptr))) Py_INCREF(self._orig_stream) Py_INCREF(self._stream) Py_INCREF(self._restorer) + Py_INCREF(self._bootstrap) Py_INCREF(self._network) self._disconnect_promise = self.on_disconnect().then(self._decref) @@ -2292,6 +2309,7 @@ cdef class TwoPartyServer: self.port_promise = Promise()._init(helpers.connectServer(deref(self._task_set), deref(self._restorer.thisptr), loop.thisptr, temp_string)) def _decref(self): + Py_DECREF(self._bootstrap) Py_DECREF(self._restorer) Py_DECREF(self._orig_stream) Py_DECREF(self._stream) @@ -2318,8 +2336,6 @@ cdef class TwoPartyServer: else: return self._port - # TODO: add restore functionality here? - cdef class _AsyncIoStream: cdef Own[AsyncIoStream] thisptr diff --git a/test/test_rpc.py b/test/test_rpc.py index 6bae298..d288d04 100644 --- a/test/test_rpc.py +++ b/test/test_rpc.py @@ -86,3 +86,17 @@ def test_ez_rpc(): with pytest.raises(capnp.KjException): response = remote.wait() + +def test_simple_rpc_bootstrap(): + read, write = socket.socketpair(socket.AF_UNIX) + + server = capnp.TwoPartyServer(write, bootstrap=Server(100)) + client = capnp.TwoPartyClient(read) + + cap = client.bootstrap() + cap = cap.cast_as(test_capability_capnp.TestInterface) + + remote = cap.foo(i=5) + response = remote.wait() + + assert response.x == '125' From 2df20212fa7e985bf086b934f100970460ce8b67 Mon Sep 17 00:00:00 2001 From: Jason Paryani Date: Tue, 24 Feb 2015 15:48:48 -0800 Subject: [PATCH 06/10] Fix possible segfault when importing multiple schemas It turns out that parseDiskFile expects the imports array to last the lifetime of the SchemaParser object. Also, optimized for the case where the imports haven't changed, so we're not allocating a crazy amount of these imports arrays. --- capnp/lib/capnp.pyx | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index 9a7efd2..6c03ee4 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -2979,6 +2979,23 @@ class _EnumModule(object): for name, val in schema.enumerants.items(): setattr(self, name, val) +cdef class _StringArrayPtr: + cdef StringPtr * thisptr + cdef object parent + cdef size_t size + + def __cinit__(self, size_t size, parent): + self.size = size + self.thisptr = malloc(sizeof(StringPtr) * size) + self.parent = parent + + def __dealloc__(self): + free(self.thisptr) + + cdef ArrayPtr[StringPtr] asArrayPtr(self) except +reraise_kj_exception: + return ArrayPtr[StringPtr](self.thisptr, self.size) + + cdef class SchemaParser: """A class for loading Cap'n Proto schema files. @@ -2986,26 +3003,34 @@ cdef class SchemaParser: """ cdef C_SchemaParser * thisptr cdef public dict modules_by_id + cdef list _all_imports + cdef _StringArrayPtr _last_import_array def __cinit__(self): self.thisptr = new C_SchemaParser() self.modules_by_id = {} + self._all_imports = [] def __dealloc__(self): del self.thisptr cpdef _parse_disk_file(self, displayName, diskPath, imports) except +reraise_kj_exception: - cdef StringPtr * importArray = malloc(sizeof(StringPtr) * len(imports)) + cdef _StringArrayPtr importArray - for i in range(len(imports)): - importArray[i] = StringPtr(imports[i]) + if self._last_import_array and self._last_import_array.parent == imports: + importArray = self._last_import_array + else: + importArray = _StringArrayPtr(len(imports), imports) - cdef ArrayPtr[StringPtr] importsPtr = ArrayPtr[StringPtr](importArray, len(imports)) + for i in range(len(imports)): + curr_import = imports[i] + importArray.thisptr[i] = StringPtr(curr_import, len(curr_import)) + + self._all_imports.append(importArray) + self._last_import_array = importArray ret = _ParsedSchema() - ret._init_child(self.thisptr.parseDiskFile(displayName, diskPath, importsPtr)) - - free(importArray) + ret._init_child(self.thisptr.parseDiskFile(displayName, diskPath, importArray.asArrayPtr())) return ret From bf05e555d2b36ce0338dce69ea403f8ba30b669a Mon Sep 17 00:00:00 2001 From: Jason Paryani Date: Mon, 2 Mar 2015 13:44:12 -0800 Subject: [PATCH 07/10] Default to using cython if cythonized files not found --- setup.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/setup.py b/setup.py index 3a467c6..890e48f 100644 --- a/setup.py +++ b/setup.py @@ -3,12 +3,6 @@ from __future__ import print_function use_cython = False -import pkg_resources -setuptools_version = pkg_resources.get_distribution("setuptools").version -if setuptools_version < '0.8': - # older versions of setuptools don't work with cython - use_cython = False - from distutils.core import setup import os import sys @@ -66,6 +60,11 @@ class clean(_clean): except OSError: pass +# set use_cython if lib/capnp.cpp is not detected +capnp_compiled_file = os.path.join(os.path.dirname(__file__), 'capnp', 'lib', 'capnp.cpp') +if not os.path.isfile(capnp_compiled_file): + use_cython = True + # hack to parse commandline arguments force_bundled_libcapnp = "--force-bundled-libcapnp" in sys.argv if force_bundled_libcapnp: From 6c847897997f0c52d436dc293a8a0a52e46b6cf4 Mon Sep 17 00:00:00 2001 From: Jason Paryani Date: Mon, 2 Mar 2015 14:11:34 -0800 Subject: [PATCH 08/10] Update bundled libcapnp to v0.5.1.1 --- buildutils/bundle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/buildutils/bundle.py b/buildutils/bundle.py index e7ba836..2e3b3e0 100644 --- a/buildutils/bundle.py +++ b/buildutils/bundle.py @@ -35,8 +35,8 @@ pjoin = os.path.join # Constants #----------------------------------------------------------------------------- -bundled_version = (0,5,1) -libcapnp = "capnproto-c++-%i.%i.%i.tar.gz" % (bundled_version) +bundled_version = (0,5,1,1) +libcapnp = "capnproto-c++-%i.%i.%i.%i.tar.gz" % (bundled_version) libcapnp_url = "https://capnproto.org/" + libcapnp HERE = os.path.dirname(__file__) From 153575d9b03e5fbd8ef43c552ca7a41f70db8037 Mon Sep 17 00:00:00 2001 From: Jason Paryani Date: Mon, 2 Mar 2015 14:15:21 -0800 Subject: [PATCH 09/10] Bump version to v0.5.4 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 890e48f..6dac4e4 100644 --- a/setup.py +++ b/setup.py @@ -14,7 +14,7 @@ _this_dir = os.path.dirname(__file__) MAJOR = 0 MINOR = 5 -MICRO = 3 +MICRO = 4 VERSION = '%d.%d.%d' % (MAJOR, MINOR, MICRO) From 782747ad2e5438e2f1ca20bb107f73f8ef8ff55a Mon Sep 17 00:00:00 2001 From: Jason Paryani Date: Mon, 2 Mar 2015 14:15:32 -0800 Subject: [PATCH 10/10] Update Changelog for v0.5.4 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81f87e5..fbc5178 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v0.5.4 (2015-03-02) +- Update bundled C++ libcapnp to v0.5.1.1 security release +- Add bootstrap RPC methods +- Fix possible segfault when importing multiple schemas + + ## v0.5.3 (2015-02-23) - Fix possible crash due to bad destructor ordering in MessageReader (by @JohnEmhoff) - Default to no longer using cython