[PATCH 1/2] python/notmuch2: do not destroy messages owned by a query

[PATCH 1/2] python/notmuch2: do not destroy messages owned by a query

[Anton Khirnov]

Any messages retrieved from a query - either directly via
search_messages() or indirectly via thread objects - are owned by that
query. Retrieving the same message (i.e. corresponding to the same
message ID / database object) several times will always yield the same
C object.

The caller is allowed to destroy message objects owned by a query before
the query itself - which can save memory for long-lived queries.
However, that message must then never be retrieved again from that
query.

The python-notmuch2 bindings will currently destroy every message object
in Message._destroy(), which will lead to an invalid free if the same
message is then retrieved again. E.g. the following python program leads
to libtalloc abort()ing:

import notmuch2
db   = notmuch2.Database(mode = notmuch2.Database.MODE.READ_ONLY)
t    = next(db.threads('*'))
msgs = list(zip(t.toplevel(), t.toplevel()))
msgs = list(zip(t.toplevel(), t.toplevel()))

Fix this issue by creating a subclass of Message, which is used for
"standalone" message which have to be freed by the caller. Message class
is then used only for messages descended from a query, which do not need
to be freed by the caller.
---
 bindings/python-cffi/notmuch2/_database.py |  6 ++--
 bindings/python-cffi/notmuch2/_message.py  | 42 ++++++++++++++--------
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
index 95f59ca0..f14eac78 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.Message(self, msg_pp[0], db=self)
+        msg = message.StandaloneMessage(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.Message(self, msg_p, db=self)
+        msg = message.StandaloneMessage(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.Message(self, msg_p, db=self)
+        msg = message.StandaloneMessage(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 c5fdbf6d..416ce7ca 100644
--- a/bindings/python-cffi/notmuch2/_message.py
+++ b/bindings/python-cffi/notmuch2/_message.py
@@ -14,7 +14,7 @@ __all__ = ['Message']
 
 
 class Message(base.NotmuchObject):
-    """An email message stored in the notmuch database.
+    """An email message stored in the notmuch database retrieved via a query.
 
     This should not be directly created, instead it will be returned
     by calling methods on :class:`Database`.  A message keeps a
@@ -61,22 +61,10 @@ class Message(base.NotmuchObject):
 
     @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
 
     @property
     def messageid(self):
@@ -375,6 +363,30 @@ 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.
+    """
+    @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()
+
+    def _destroy(self):
+        if self.alive:
+            capi.lib.notmuch_message_destroy(self._msg_p)
+        self._msg_p = None
 
 class FilenamesIter(base.NotmuchIter):
     """Iterator for binary filenames objects."""
-- 
2.20.1

Re: [PATCH 1/2] python/notmuch2: do not destroy messages owned by a query

[Anton Khirnov]

ping

-- 
Anton Khirnov

Re: [PATCH 1/2] python/notmuch2: do not destroy messages owned by a query

[David Bremner]

Anton Khirnov <anton@khirnov.net> writes:

> ping

I'm hoping Floris will find a chance to review your patches.

d

Re: [PATCH 1/2] python/notmuch2: do not destroy messages owned by a query

[Floris Bruynooghe]

Hi Anton,

Thanks for improving the bindings!  Any my apologies for the late
response, I failed to spot this mail the first time round.

Also, this is a pretty serious bug, thanks for finding it.

This looks pretty solid, a few small style comments that aren't very
important notwithstanding.  Though I do think you should be able to add
a test for this in the pytest tests.  I think just triggering the
use-after-free you demonstrate in the commit message is fine as it will
work with your patch.  The fact it will crash rather then fail without
is unfortunate but probably fine.

On Sat 09 May 2020 at 07:05 +0200, Anton Khirnov wrote:

> Any messages retrieved from a query - either directly via
> search_messages() or indirectly via thread objects - are owned by that
> query. Retrieving the same message (i.e. corresponding to the same
> message ID / database object) several times will always yield the same
> C object.
>
> The caller is allowed to destroy message objects owned by a query before
> the query itself - which can save memory for long-lived queries.
> However, that message must then never be retrieved again from that
> query.
>
> The python-notmuch2 bindings will currently destroy every message object
> in Message._destroy(), which will lead to an invalid free if the same
> message is then retrieved again. E.g. the following python program leads
> to libtalloc abort()ing:
>
> import notmuch2
> db   = notmuch2.Database(mode = notmuch2.Database.MODE.READ_ONLY)
> t    = next(db.threads('*'))
> msgs = list(zip(t.toplevel(), t.toplevel()))
> msgs = list(zip(t.toplevel(), t.toplevel()))
>
> Fix this issue by creating a subclass of Message, which is used for
> "standalone" message which have to be freed by the caller. Message class
> is then used only for messages descended from a query, which do not need
> to be freed by the caller.
> ---
>  bindings/python-cffi/notmuch2/_database.py |  6 ++--
>  bindings/python-cffi/notmuch2/_message.py  | 42 ++++++++++++++--------
>  2 files changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
> index 95f59ca0..f14eac78 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.Message(self, msg_pp[0], db=self)
> +        msg = message.StandaloneMessage(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.Message(self, msg_p, db=self)
> +        msg = message.StandaloneMessage(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.Message(self, msg_p, db=self)
> +        msg = message.StandaloneMessage(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 c5fdbf6d..416ce7ca 100644
> --- a/bindings/python-cffi/notmuch2/_message.py
> +++ b/bindings/python-cffi/notmuch2/_message.py
> @@ -14,7 +14,7 @@ __all__ = ['Message']
>  
>  
>  class Message(base.NotmuchObject):
> -    """An email message stored in the notmuch database.
> +    """An email message stored in the notmuch database retrieved via a query.
>  
>      This should not be directly created, instead it will be returned
>      by calling methods on :class:`Database`.  A message keeps a
> @@ -61,22 +61,10 @@ class Message(base.NotmuchObject):
>  
>      @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

I feel like some more description from the commit message could live
here to explain why this isn't destroying the message rather than having
to find it in the commit message.

>  
>      @property
>      def messageid(self):
> @@ -375,6 +363,30 @@ class Message(base.NotmuchObject):
>          if isinstance(other, self.__class__):
>              return self.messageid == other.messageid
>  
> +class StandaloneMessage(Message):

Minor style thing, between top-level code one usually leaves 2 blank
lines per PEP8.  Nowadays perhaps we should think about adopting an
automatic formatter but hey.

(This applies to lots of places in both patches btw)

> +    """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.
> +    """

Likewise I'd be tempted to add here that this message is destroyed as
soon as it is garbage collected pointing out that this is not the case
for Message.  And probably update the Message docstring with the inverse.

> +    @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()
> +
> +    def _destroy(self):
> +        if self.alive:
> +            capi.lib.notmuch_message_destroy(self._msg_p)
> +        self._msg_p = None
>  
>  class FilenamesIter(base.NotmuchIter):
>      """Iterator for binary filenames objects."""

Re: [PATCH 1/2] python/notmuch2: do not destroy messages owned by a query

[Tomi Ollila]

Hi Anton,

On Thu, May 21 2020, Anton Khirnov wrote:

> ping

Great (an important) stuff in this series !

...just that...

Floris did good review on your changes the next day.

What I remember was there (looked 30 mins ago)

- style: somewhat important (I press more there). Easy to get better,
         good and consistent style makes it better

- docs in code: like Floris mentioned, the docs in commit messages are
                are not seen by user. The relevant documentation is 
                to be with the code. commit message tells what was done
                and why

- tests: since pytest is used to test the current notmuch2 cffi python
         tests, all new code that can be tested pytest code to do so
         is to be added

And these have to be done now -- postponing such a things into future
will just mean that it never happens, and the messines and entropy of
the notmuch code gets even greater in such a case...

>
> -- 
> Anton Khirnov

Tomi

python: Continuing message re-use fix

[Floris Bruynooghe]

Hi,

This builds on the patch by Anton Khirnov to fix the message re-use
that is possible when accessing messages from a thread.  I started
with just addressing my own comments on this patch, but evolved it
into switching the logic around and leave the normal Message object
untouched.  Instead I created a new OwnedMessage which is used by
the Thread which does not free itself on __del__().  I think this
is preferable because the other iterators, mainly Database.messages(),
do not allow retrieving messages more than once since the query object
is hidden from the API.

I've left the original commit in this patch series to not alter any
contributions.

Cheers,
Floris

[PATCH 1/2] python/notmuch2: do not destroy messages owned by a query

[Floris Bruynooghe]

From: Anton Khirnov <anton@khirnov.net>

Any messages retrieved from a query - either directly via
search_messages() or indirectly via thread objects - are owned by that
query. Retrieving the same message (i.e. corresponding to the same
message ID / database object) several times will always yield the same
C object.

The caller is allowed to destroy message objects owned by a query before
the query itself - which can save memory for long-lived queries.
However, that message must then never be retrieved again from that
query.

The python-notmuch2 bindings will currently destroy every message object
in Message._destroy(), which will lead to an invalid free if the same
message is then retrieved again. E.g. the following python program leads
to libtalloc abort()ing:

import notmuch2
db   = notmuch2.Database(mode = notmuch2.Database.MODE.READ_ONLY)
t    = next(db.threads('*'))
msgs = list(zip(t.toplevel(), t.toplevel()))
msgs = list(zip(t.toplevel(), t.toplevel()))

Fix this issue by creating a subclass of Message, which is used for
"standalone" message which have to be freed by the caller. Message class
is then used only for messages descended from a query, which do not need
to be freed by the caller.
---
 bindings/python-cffi/notmuch2/_database.py |  6 ++--
 bindings/python-cffi/notmuch2/_message.py  | 42 ++++++++++++++--------
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
index 95f59ca0..f14eac78 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.Message(self, msg_pp[0], db=self)
+        msg = message.StandaloneMessage(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.Message(self, msg_p, db=self)
+        msg = message.StandaloneMessage(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.Message(self, msg_p, db=self)
+        msg = message.StandaloneMessage(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 c5fdbf6d..416ce7ca 100644
--- a/bindings/python-cffi/notmuch2/_message.py
+++ b/bindings/python-cffi/notmuch2/_message.py
@@ -14,7 +14,7 @@ __all__ = ['Message']
 
 
 class Message(base.NotmuchObject):
-    """An email message stored in the notmuch database.
+    """An email message stored in the notmuch database retrieved via a query.
 
     This should not be directly created, instead it will be returned
     by calling methods on :class:`Database`.  A message keeps a
@@ -61,22 +61,10 @@ class Message(base.NotmuchObject):
 
     @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
 
     @property
     def messageid(self):
@@ -375,6 +363,30 @@ 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.
+    """
+    @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()
+
+    def _destroy(self):
+        if self.alive:
+            capi.lib.notmuch_message_destroy(self._msg_p)
+        self._msg_p = None
 
 class FilenamesIter(base.NotmuchIter):
     """Iterator for binary filenames objects."""
-- 
2.27.0

[PATCH 2/2] Make messages returned by Thread objects owned

[Floris Bruynooghe]

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

Re: python: Continuing message re-use fix

[David Bremner]

Floris Bruynooghe <flub@devork.be> writes:

> Hi,
>
> This builds on the patch by Anton Khirnov to fix the message re-use
> that is possible when accessing messages from a thread.  I started
> with just addressing my own comments on this patch, but evolved it
> into switching the logic around and leave the normal Message object
> untouched.  Instead I created a new OwnedMessage which is used by
> the Thread which does not free itself on __del__().  I think this
> is preferable because the other iterators, mainly Database.messages(),
> do not allow retrieving messages more than once since the query object
> is hidden from the API.
>
> I've left the original commit in this patch series to not alter any
> contributions.
>
> Cheers,
> Floris

Revised series merged to master and release.

Thanks for your help with these updates.

d