From d9a2b900b6525874b913276af91840983d81b3f1 Mon Sep 17 00:00:00 2001 From: David Bremner Date: Sat, 8 Jan 2022 10:03:15 -0400 Subject: [PATCH 1/2] test: add known broken tests for recursive traversal of replies. This reproduces the bug reported at [1]. The second test hints at the solution, making reply return OwnedMessage objects. [1]: id:87sfu6utxg.fsf@tethera.net --- test/T392-python-cffi-notmuch.sh | 50 ++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100755 test/T392-python-cffi-notmuch.sh diff --git a/test/T392-python-cffi-notmuch.sh b/test/T392-python-cffi-notmuch.sh new file mode 100755 index 00000000..50012c55 --- /dev/null +++ b/test/T392-python-cffi-notmuch.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +test_description="python bindings (notmuch test suite)" +. $(dirname "$0")/test-lib.sh || exit 1 + +if [ $NOTMUCH_HAVE_PYTHON3_CFFI -eq 0 -o $NOTMUCH_HAVE_PYTHON3_PYTEST -eq 0 ]; then + test_done +fi + +add_email_corpus + +cat < recurse.py +from notmuch2 import Database +def show_msgs(msgs, level): + print('{:s} {:s}'.format(' ' * level*4, type(msgs).__name__)) + for msg in msgs: + print('{:s} {:s}'.format(' ' * (level*4+2), type(msg).__name__)) + replies=msg.replies() + show_msgs(replies, level+1) +db = Database(config=Database.CONFIG.SEARCH) +msg=db.find("87ocn0qh6d.fsf@yoom.home.cworth.org") +threads = db.threads(query="thread:"+msg.threadid) +thread = next (threads) +show_msgs(thread, 0) +EOF + +test_begin_subtest "recursive traversal of replies (no crash)" +test_subtest_known_broken +test_python < recurse.py +error=$? +test_expect_equal "${error}" 0 + +test_begin_subtest "recursive traversal of replies (output)" +test_subtest_known_broken +test_python < recurse.py +tail -n 10 < OUTPUT > OUTPUT.sample +cat < EXPECTED + OwnedMessage + MessageIter + OwnedMessage + MessageIter + OwnedMessage + MessageIter + OwnedMessage + MessageIter + OwnedMessage + MessageIter +EOF +test_expect_equal_file EXPECTED OUTPUT.sample + +test_done From 9e7ea628e6bddbd7345d053a3daf14af74896cc2 Mon Sep 17 00:00:00 2001 From: David Bremner Date: Sat, 8 Jan 2022 10:03:16 -0400 Subject: [PATCH 2/2] python-cffi: returned OwnedMessage objects from Message.replies If we return regular Message objects, python will try to destroy them, and the underlying notmuch object, causing e.g. the crash [1]. [1]: id:87sfu6utxg.fsf@tethera.net --- bindings/python-cffi/notmuch2/_message.py | 4 ++-- test/T392-python-cffi-notmuch.sh | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/bindings/python-cffi/notmuch2/_message.py b/bindings/python-cffi/notmuch2/_message.py index 2f232076..b4f651fb 100644 --- a/bindings/python-cffi/notmuch2/_message.py +++ b/bindings/python-cffi/notmuch2/_message.py @@ -357,14 +357,14 @@ class Message(base.NotmuchObject): This method will only work if the message was created from a thread. Otherwise it will yield no results. - :returns: An iterator yielding :class:`Message` instances. + :returns: An iterator yielding :class:`OwnedMessage` instances. :rtype: MessageIter """ # The notmuch_messages_valid call accepts NULL and this will # become an empty iterator, raising StopIteration immediately. # Hence no return value checking here. msgs_p = capi.lib.notmuch_message_get_replies(self._msg_p) - return MessageIter(self, msgs_p, db=self._db) + return MessageIter(self, msgs_p, db=self._db, msg_cls=OwnedMessage) def __hash__(self): return hash(self.messageid) diff --git a/test/T392-python-cffi-notmuch.sh b/test/T392-python-cffi-notmuch.sh index 50012c55..15c8fc6b 100755 --- a/test/T392-python-cffi-notmuch.sh +++ b/test/T392-python-cffi-notmuch.sh @@ -24,13 +24,11 @@ show_msgs(thread, 0) EOF test_begin_subtest "recursive traversal of replies (no crash)" -test_subtest_known_broken test_python < recurse.py error=$? test_expect_equal "${error}" 0 test_begin_subtest "recursive traversal of replies (output)" -test_subtest_known_broken test_python < recurse.py tail -n 10 < OUTPUT > OUTPUT.sample cat < EXPECTED