Make messages returned by Thread objects owned

This reverses the logic of StandaloneMessage to instead create a
OwnedMessage.  Only the Thread class allows retrieving messages more
then once so it can explicitly create such messages.

The added test fails with SIGABRT without the fix for the message
re-use in threads being present.

bindings/python-cffi/notmuch2/_database.py

@@ -400,7 +400,7 @@ class Database(base.NotmuchObject):
               capi.lib.NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID]
         if ret not in ok:
             raise errors.NotmuchError(ret)
-        msg = message.StandaloneMessage(self, msg_pp[0], db=self)
+        msg = message.Message(self, msg_pp[0], db=self)
         if sync_flags:
             msg.tags.from_maildir_flags()
         return self.AddedMessage(
@@ -469,7 +469,7 @@ class Database(base.NotmuchObject):
         msg_p = msg_pp[0]
         if msg_p == capi.ffi.NULL:
             raise LookupError
-        msg = message.StandaloneMessage(self, msg_p, db=self)
+        msg = message.Message(self, msg_p, db=self)
         return msg
 
     def get(self, filename):
@@ -502,7 +502,7 @@ class Database(base.NotmuchObject):
         msg_p = msg_pp[0]
         if msg_p == capi.ffi.NULL:
             raise LookupError
-        msg = message.StandaloneMessage(self, msg_p, db=self)
+        msg = message.Message(self, msg_p, db=self)
         return msg
 
     @property

bindings/python-cffi/notmuch2/_message.py

@@ -47,9 +47,7 @@ class Message(base.NotmuchObject):
     :type db: Database
     :param msg_p: The C pointer to the ``notmuch_message_t``.
     :type msg_p: <cdata>
-
     :param dup: Whether the message was a duplicate on insertion.
-
     :type dup: None or bool
     """
     _msg_p = base.MemoryPointer()
@@ -61,10 +59,22 @@ class Message(base.NotmuchObject):
 
     @property
     def alive(self):
-        return self._parent.alive
+        if not self._parent.alive:
+            return False
+        try:
+            self._msg_p
+        except errors.ObjectDestroyedError:
+            return False
+        else:
+            return True
+
+    def __del__(self):
+        self._destroy()
 
     def _destroy(self):
-        pass
+        if self.alive:
+            capi.lib.notmuch_message_destroy(self._msg_p)
+        self._msg_p = None
 
     @property
     def messageid(self):
@@ -363,30 +373,26 @@ class Message(base.NotmuchObject):
         if isinstance(other, self.__class__):
             return self.messageid == other.messageid
 
-class StandaloneMessage(Message):
-    """An email message stored in the notmuch database.
 
-    This subclass of Message is used for messages that are retrieved from the
+class OwnedMessage(Message):
-    database directly and are not owned by a query.
+    """An email message owned by parent thread object.
+
+    This subclass of Message is used for messages that are retrieved
+    from the notmuch database via a parent :class:`notmuch2.Thread`
+    object, which "owns" this message.  This means that when this
+    message object is destroyed, by calling :func:`del` or
+    :meth:`_destroy` directly or indirectly, the message is not freed
+    in the notmuch API and the parent :class:`notmuch2.Thread` object
+    can return the same object again when needed.
     """
+
     @property
     def alive(self):
-        if not self._parent.alive:
+        return self._parent.alive
-            return False
-        try:
-            self._msg_p
-        except errors.ObjectDestroyedError:
-            return False
-        else:
-            return True
-
-    def __del__(self):
-        self._destroy()
 
     def _destroy(self):
-        if self.alive:
+        pass
-            capi.lib.notmuch_message_destroy(self._msg_p)
+
-        self._msg_p = None
 
 class FilenamesIter(base.NotmuchIter):
     """Iterator for binary filenames objects."""
@@ -690,8 +696,9 @@ collections.abc.ValuesView.register(PropertiesValuesView)
 
 class MessageIter(base.NotmuchIter):
 
-    def __init__(self, parent, msgs_p, *, db):
+    def __init__(self, parent, msgs_p, *, db, msg_cls=Message):
         self._db = db
+        self._msg_cls = msg_cls
         super().__init__(parent, msgs_p,
                          fn_destroy=capi.lib.notmuch_messages_destroy,
                          fn_valid=capi.lib.notmuch_messages_valid,
@@ -700,4 +707,4 @@ class MessageIter(base.NotmuchIter):
 
     def __next__(self):
         msg_p = super().__next__()
-        return Message(self, msg_p, db=self._db)
+        return self._msg_cls(self, msg_p, db=self._db)

bindings/python-cffi/notmuch2/_thread.py

@@ -62,7 +62,9 @@ class Thread(base.NotmuchObject, collections.abc.Iterable):
         :raises ObjectDestroyedError: if used after destroyed.
         """
         msgs_p = capi.lib.notmuch_thread_get_toplevel_messages(self._thread_p)
-        return message.MessageIter(self, msgs_p, db=self._db)
+        return message.MessageIter(self, msgs_p,
+                                   db=self._db,
+                                   msg_cls=message.OwnedMessage)
 
     def __iter__(self):
         """Return an iterator over all the messages in the thread.
@@ -72,7 +74,9 @@ class Thread(base.NotmuchObject, collections.abc.Iterable):
         :raises ObjectDestroyedError: if used after destroyed.
         """
         msgs_p = capi.lib.notmuch_thread_get_messages(self._thread_p)
-        return message.MessageIter(self, msgs_p, db=self._db)
+        return message.MessageIter(self, msgs_p,
+                                   db=self._db,
+                                   msg_cls=message.OwnedMessage)
 
     @property
     def matched(self):

bindings/python-cffi/tests/test_database.py

@@ -324,3 +324,14 @@ class TestQuery:
         threads = db.threads('*')
         thread = next(threads)
         assert isinstance(thread, notmuch2.Thread)
+
+    def test_use_threaded_message_twice(self, db):
+        thread = next(db.threads('*'))
+        for msg in thread.toplevel():
+            assert isinstance(msg, notmuch2.Message)
+            assert msg.alive
+            del msg
+        for msg in thread:
+            assert isinstance(msg, notmuch2.Message)
+            assert msg.alive
+            del msg