[PATCH 2/2] Make messages returned by Thread objects owned
Floris Bruynooghe
flub at devork.be
Mon Jun 15 13:58:50 PDT 2020
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 | 6 +--
bindings/python-cffi/notmuch2/_message.py | 55 ++++++++++++---------
bindings/python-cffi/notmuch2/_thread.py | 8 ++-
bindings/python-cffi/tests/test_database.py | 11 +++++
4 files changed, 51 insertions(+), 29 deletions(-)
diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
index f14eac78..95f59ca0 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -399,7 +399,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(
@@ -468,7 +468,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):
@@ -501,7 +501,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
diff --git a/bindings/python-cffi/notmuch2/_message.py b/bindings/python-cffi/notmuch2/_message.py
index 416ce7ca..02de50ad 100644
--- a/bindings/python-cffi/notmuch2/_message.py
+++ b/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
- database directly and are not owned by a query.
+class OwnedMessage(Message):
+ """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 False
- try:
- self._msg_p
- except errors.ObjectDestroyedError:
- return False
- else:
- return True
-
- def __del__(self):
- self._destroy()
+ return self._parent.alive
def _destroy(self):
- if self.alive:
- capi.lib.notmuch_message_destroy(self._msg_p)
- self._msg_p = None
+ pass
+
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)
diff --git a/bindings/python-cffi/notmuch2/_thread.py b/bindings/python-cffi/notmuch2/_thread.py
index bb76f2dc..e883f308 100644
--- a/bindings/python-cffi/notmuch2/_thread.py
+++ b/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):
diff --git a/bindings/python-cffi/tests/test_database.py b/bindings/python-cffi/tests/test_database.py
index e3a8344d..55069b5e 100644
--- a/bindings/python-cffi/tests/test_database.py
+++ b/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
--
2.27.0
More information about the notmuch
mailing list