From 927d8b5128282bf38a1f109d8b0a7c2be0141f71 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Thu, 1 Apr 2021 07:31:10 +0800 Subject: [PATCH 1/7] tox.ini: Use skipdist for faster recreate --- tox.ini | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index 75af016..0c1fa44 100644 --- a/tox.ini +++ b/tox.ini @@ -1,13 +1,18 @@ [tox] envlist = py27,py34,py35,py36 +skipsdist = True [testenv] deps= + pkgconfig + Jinja2 pytest cython commands = - py.test + python setup.py install + py.test {posargs} setenv = - CFLAGS='-stdlib=libc++' \ No newline at end of file + CFLAGS='-stdlib=libc++' + CXXFLAGS='-stdlib=libc++' From 0f6df849cd40a5371afa88f62ac3df60a9633d83 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Tue, 1 Jun 2021 15:40:35 +0800 Subject: [PATCH 2/7] Refine exception invoking which on non-union type Related to https://github.com/capnproto/pycapnp/issues/254 --- capnp/lib/capnp.pyx | 24 ++++++++++++++++-------- test/test_struct.py | 22 ++++++++++++++++------ 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index 42f96c0..f01a63a 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -1147,8 +1147,10 @@ cdef class _DynamicStructReader: cpdef _which_str(self): try: return helpers.fixMaybe(self.thisptr.which()).getProto().getName().cStr() - except: - raise KjException("Attempted to call which on a non-union type") + except RuntimeError as e: + if str(e) == "Member was null.": + raise KjException("Attempted to call which on a non-union type") + raise cpdef _DynamicEnumField _which(self): """Returns the enum corresponding to the union in this struct @@ -1161,8 +1163,10 @@ cdef class _DynamicStructReader: try: which = _DynamicEnumField()._init( _StructSchemaField()._init(helpers.fixMaybe(self.thisptr.which()), self).proto) - except: - raise KjException("Attempted to call which on a non-union type") + except RuntimeError as e: + if str(e) == "Member was null.": + raise KjException("Attempted to call which on a non-union type") + raise return which @@ -1445,8 +1449,10 @@ cdef class _DynamicStructBuilder: cpdef _which_str(self): try: return helpers.fixMaybe(self.thisptr.which()).getProto().getName().cStr() - except: - raise KjException("Attempted to call which on a non-union type") + except RuntimeError as e: + if str(e) == "Member was null.": + raise KjException("Attempted to call which on a non-union type") + raise cpdef _DynamicEnumField _which(self): """Returns the enum corresponding to the union in this struct @@ -1459,8 +1465,10 @@ cdef class _DynamicStructBuilder: try: which = _DynamicEnumField()._init( _StructSchemaField()._init(helpers.fixMaybe(self.thisptr.which()), self).proto) - except: - raise KjException("Attempted to call which on a non-union type") + except RuntimeError as e: + if str(e) == "Member was null.": + raise KjException("Attempted to call which on a non-union type") + raise return which diff --git a/test/test_struct.py b/test/test_struct.py index 8dca0fd..153e003 100644 --- a/test/test_struct.py +++ b/test/test_struct.py @@ -4,6 +4,8 @@ import os import tempfile import sys +from capnp.lib.capnp import KjException + this_dir = os.path.dirname(__file__) @@ -37,9 +39,13 @@ def test_which_builder(addressbook): assert bob.employment.which == addressbook.Person.Employment.unemployed assert bob.employment.which == "unemployed" - with pytest.raises(Exception): - addresses.which - with pytest.raises(Exception): + with pytest.raises(KjException): + addresses._which() + + with pytest.raises(KjException): + addresses._which_str() + + with pytest.raises(KjException): addresses.which @@ -71,9 +77,13 @@ def test_which_reader(addressbook): bob = people[1] assert bob.employment.which == "unemployed" - with pytest.raises(Exception): - addresses.which - with pytest.raises(Exception): + with pytest.raises(KjException): + addresses._which_str() + + with pytest.raises(KjException): + addresses._which() + + with pytest.raises(KjException): addresses.which From a20e69dbe9e251e867ca7c12b47ee964e5c09708 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Tue, 1 Jun 2021 15:42:17 +0800 Subject: [PATCH 3/7] Replace bare exception from getfullargspec getfullargspec is based on signature, which is documented to raise TypeError and ValueError. Related to https://github.com/capnproto/pycapnp/issues/254 --- capnp/lib/capnp.pyx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index f01a63a..0ff15a4 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -1974,7 +1974,7 @@ cdef class _Promise: argspec = None try: argspec = _inspect.getfullargspec(func) - except: + except (TypeError, ValueError): pass if argspec: args_length = len(argspec.args) if argspec.args else 0 @@ -2042,7 +2042,7 @@ cdef class _VoidPromise: argspec = None try: argspec = _inspect.getfullargspec(func) - except: + except (TypeError, ValueError): pass if argspec: args_length = len(argspec.args) if argspec.args else 0 @@ -2146,7 +2146,7 @@ cdef class _RemotePromise: argspec = None try: argspec = _inspect.getfullargspec(func) - except: + except (TypeError, ValueError): pass if argspec: args_length = len(argspec.args) if argspec.args else 0 From bd91c52d57f391007b0e294e5ee2a7fb4150f3c6 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Tue, 1 Jun 2021 15:51:23 +0800 Subject: [PATCH 4/7] Fix bare exception from str(obj) There is no test coverage for these exception clauses, however the invocation of obj.__str__() for client objects could raise any exception, hence the very broad exception catch. --- capnp/lib/capnp.pyx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index 0ff15a4..93dd21a 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -89,7 +89,7 @@ cdef api VoidPromise * call_server_method(PyObject * _server, warning_msg = ( "Server function ({}) returned a value that was not a Promise: return = {}" .format(method_name, str(ret))) - except: + except Exception: warning_msg = 'Server function (%s) returned a value that was not a Promise' % (method_name) _warnings.warn_explicit( warning_msg, UserWarning, _inspect.getsourcefile(func), _inspect.getsourcelines(func)[1]) @@ -104,7 +104,7 @@ cdef api VoidPromise * call_server_method(PyObject * _server, warning_msg = ( "Server function ({}) returned a value that was not a Promise: return = {}" .format(method_name, str(ret))) - except: + except Exception: warning_msg = 'Server function (%s) returned a value that was not a Promise' % (method_name) _warnings.warn_explicit( warning_msg, UserWarning, _inspect.getsourcefile(func), _inspect.getsourcelines(func)[1]) @@ -289,7 +289,7 @@ cdef api object get_exception_info(object exc_type, object exc_obj, object exc_t return (exc_tb.tb_frame.f_code.co_filename.encode(), exc_tb.tb_lineno, (repr(exc_type) + ":" + str(exc_obj)).encode()) - except: + except Exception: return (b'', 0, b"Couldn't determine python exception") From 479765432391ecb62c95f82cef0b8db0ce00e288 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Tue, 1 Jun 2021 16:27:34 +0800 Subject: [PATCH 5/7] remove_event_loop: Allow ignoring specific errors Replaces a bare except with Exception, and updates the test case to specify only a single exception that is allowed to occur. Related to https://github.com/capnproto/pycapnp/issues/254 --- capnp/lib/capnp.pyx | 18 ++++++++++++++---- test/test_threads.py | 17 ++++++++++++++--- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index 93dd21a..1f60975 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -1843,16 +1843,26 @@ cpdef remove_event_loop(ignore_errors=False): if C_DEFAULT_EVENT_LOOP: try: C_DEFAULT_EVENT_LOOP._remove() - except: - if not ignore_errors: + except Exception as e: + if isinstance(ignore_errors, Exception): + if isinstance(e, ignore_errors): + ignore_errors = True + if ignore_errors is True: + pass + else: raise C_DEFAULT_EVENT_LOOP = None if len(_THREAD_LOCAL_EVENT_LOOPS) > 0: for loop in _THREAD_LOCAL_EVENT_LOOPS: try: loop._remove() - except: - if not ignore_errors: + except Exception as e: + if isinstance(ignore_errors, Exception): + if isinstance(e, ignore_errors): + ignore_errors = True + if ignore_errors is True: + pass + else: raise _THREAD_LOCAL_EVENT_LOOPS = [] _C_DEFAULT_EVENT_LOOP_LOCAL = None diff --git a/test/test_threads.py b/test/test_threads.py index 6aede31..be619b2 100644 --- a/test/test_threads.py +++ b/test/test_threads.py @@ -9,6 +9,9 @@ import threading import pytest import capnp + +from capnp.lib.capnp import KjException + import test_capability_capnp @@ -35,11 +38,19 @@ def test_making_threaded_event_loop(): ''' Threaded event loop test ''' - capnp.remove_event_loop(True) - capnp.create_event_loop(True) + # The following raises a KjException, and if not caught causes an SIGABRT: + # kj/async.c++:973: failed: expected head == nullptr; EventLoop destroyed with events still in the queue. + # Memory leak?; head->trace() = kj::_::ForkHub + # kj::_::AdapterPromiseNode > + # stack: ... + # python(..) malloc: *** error for object 0x...: pointer being freed was not allocated + # python(..) malloc: *** set a breakpoint in malloc_error_break to debug + # Fatal Python error: Aborted + capnp.remove_event_loop(KjException) + capnp.create_event_loop(KjException) capnp.remove_event_loop() - capnp.create_event_loop(True) + capnp.create_event_loop(KjException) class Server(test_capability_capnp.TestInterface.Server): From 2850b3792da4ab9cd79daa76c11922b044bd076c Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Tue, 1 Jun 2021 16:31:14 +0800 Subject: [PATCH 6/7] packagingtest.yml: Prevent bare except Fixes https://github.com/capnproto/pycapnp/issues/254 --- .github/workflows/packagingtest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/packagingtest.yml b/.github/workflows/packagingtest.yml index f200722..49e55ef 100644 --- a/.github/workflows/packagingtest.yml +++ b/.github/workflows/packagingtest.yml @@ -32,7 +32,7 @@ jobs: - name: Lint with flake8 run: | pip install flake8 - flake8 . --filename '*.py,*.pyx,*.pxd' --count --max-complexity=10 --max-line-length=120 --ignore=E211,E225,E226,E227,E231,E251,E261,E262,E265,E402,E722,E999 --show-source --statistics --exclude benchmark,build,capnp/templates/module.pyx + flake8 . --filename '*.py,*.pyx,*.pxd' --count --max-complexity=10 --max-line-length=120 --ignore=E211,E225,E226,E227,E231,E251,E261,E262,E265,E402,E999 --show-source --statistics --exclude benchmark,build,capnp/templates/module.pyx flake8 . --count --max-complexity=10 --max-line-length=120 --show-source --statistics --exclude benchmark,build - name: Packaging run: | From 91550e5533df743e5f36b2df223aacc9ae2ad958 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Wed, 2 Jun 2021 03:06:05 +0800 Subject: [PATCH 7/7] tox.ini: Update supported Pythons --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 0c1fa44..c72ec14 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py27,py34,py35,py36 +envlist = py37,py38,py39 skipsdist = True [testenv]