From 2ea023a3b56ee76081ae182030c399399c14ef7b Mon Sep 17 00:00:00 2001 From: davfsa Date: Sun, 19 Jun 2022 10:39:14 +0200 Subject: [PATCH 1/4] Keep message reference when updating cache messages - Deserialize `referenced_message` as the partial message it is --- changes/idk.bugfix.md | 2 ++ hikari/impl/cache.py | 11 ++++++++++- hikari/impl/entity_factory.py | 4 ++-- hikari/messages.py | 9 ++++++--- tests/hikari/impl/test_entity_factory.py | 2 +- 5 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 changes/idk.bugfix.md diff --git a/changes/idk.bugfix.md b/changes/idk.bugfix.md new file mode 100644 index 0000000000..93dcaedc01 --- /dev/null +++ b/changes/idk.bugfix.md @@ -0,0 +1,2 @@ +Properly garbage collect message references in the cache + - properly deserialize `PartialMessage.referenced_message` as the partial message it is diff --git a/hikari/impl/cache.py b/hikari/impl/cache.py index 5d071cb4df..d2022f6273 100644 --- a/hikari/impl/cache.py +++ b/hikari/impl/cache.py @@ -1563,7 +1563,16 @@ def _set_message( referenced_message: typing.Optional[cache_utility.RefCell[cache_utility.MessageData]] = None if message.referenced_message: - referenced_message = self._set_message(message.referenced_message) + reference_id = message.referenced_message.id + + if referenced_message := ( + self._message_entries.get(reference_id) or self._referenced_messages.get(reference_id) + ): + # We just update the already cached message to prevent losing the link between referenced messages, + # which could lead to a memory leak when garbage collecting + referenced_message.object.update(message.referenced_message) + else: + referenced_message = self._set_message(message.referenced_message) # Only increment ref counts if this wasn't previously cached. if message.id not in self._referenced_messages and message.id not in self._message_entries: diff --git a/hikari/impl/entity_factory.py b/hikari/impl/entity_factory.py index bee6eff4a1..ee821b82b7 100644 --- a/hikari/impl/entity_factory.py +++ b/hikari/impl/entity_factory.py @@ -2514,9 +2514,9 @@ def deserialize_message( if "message_reference" in payload: message_reference = self._deserialize_message_reference(payload["message_reference"]) - referenced_message: typing.Optional[message_models.Message] = None + referenced_message: typing.Optional[message_models.PartialMessage] = None if referenced_message_payload := payload.get("referenced_message"): - referenced_message = self.deserialize_message(referenced_message_payload) + referenced_message = self.deserialize_partial_message(referenced_message_payload) application: typing.Optional[message_models.MessageApplication] = None if "application" in payload: diff --git a/hikari/messages.py b/hikari/messages.py index 923283e939..9c98b17e0e 100644 --- a/hikari/messages.py +++ b/hikari/messages.py @@ -895,7 +895,7 @@ class PartialMessage(snowflakes.Unique): This is a string used for validating a message was sent. """ - referenced_message: undefined.UndefinedNoneOr[Message] = attr.field(hash=False, eq=False, repr=False) + referenced_message: undefined.UndefinedNoneOr[PartialMessage] = attr.field(hash=False, eq=False, repr=False) """The message that was replied to. If `type` is `MessageType.REPLY` and `hikari.undefined.UNDEFINED`, Discord's @@ -1737,8 +1737,11 @@ class Message(PartialMessage): nonce: typing.Optional[str] = attr.field(hash=False, eq=False, repr=False) """The message nonce. This is a string used for validating a message was sent.""" - referenced_message: typing.Optional[Message] = attr.field(hash=False, eq=False, repr=False) - """The message that was replied to.""" + referenced_message: typing.Optional[PartialMessage] = attr.field(hash=False, eq=False, repr=False) + """The message that was replied to. + + If `type` is `MessageType.REPLY` and `builtins.None`, the message was deleted. + """ interaction: typing.Optional[MessageInteraction] = attr.field(hash=False, eq=False, repr=False) """Information about the interaction this message was created by.""" diff --git a/tests/hikari/impl/test_entity_factory.py b/tests/hikari/impl/test_entity_factory.py index b8e449ff6f..0800ef60ba 100644 --- a/tests/hikari/impl/test_entity_factory.py +++ b/tests/hikari/impl/test_entity_factory.py @@ -4818,7 +4818,7 @@ def test_deserialize_message( assert message.message_reference.guild_id == 278325129692446720 assert isinstance(message.message_reference, message_models.MessageReference) - assert message.referenced_message == entity_factory_impl.deserialize_message(referenced_message) + assert message.referenced_message == entity_factory_impl.deserialize_partial_message(referenced_message) assert message.flags == message_models.MessageFlag.IS_CROSSPOST # Sticker From 0f7f06e1fd8d2620bdc4dd83038b93a7c3f9a52e Mon Sep 17 00:00:00 2001 From: davfsa Date: Sun, 19 Jun 2022 10:50:58 +0200 Subject: [PATCH 2/4] Rename fragment --- changes/{idk.bugfix.md => 1192.bugfix.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changes/{idk.bugfix.md => 1192.bugfix.md} (100%) diff --git a/changes/idk.bugfix.md b/changes/1192.bugfix.md similarity index 100% rename from changes/idk.bugfix.md rename to changes/1192.bugfix.md From d3f35b46d12f860afee65329a5fb5e3c4e7e1a86 Mon Sep 17 00:00:00 2001 From: davfsa Date: Sun, 19 Jun 2022 11:02:15 +0200 Subject: [PATCH 3/4] Remove trailing whitespace --- hikari/messages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hikari/messages.py b/hikari/messages.py index 9c98b17e0e..cc647e9865 100644 --- a/hikari/messages.py +++ b/hikari/messages.py @@ -1739,7 +1739,7 @@ class Message(PartialMessage): referenced_message: typing.Optional[PartialMessage] = attr.field(hash=False, eq=False, repr=False) """The message that was replied to. - + If `type` is `MessageType.REPLY` and `builtins.None`, the message was deleted. """ From aa9a194a2dfb15733e34f6ece1cdb71d4f4365f6 Mon Sep 17 00:00:00 2001 From: davfsa Date: Sun, 19 Jun 2022 11:48:15 +0200 Subject: [PATCH 4/4] Fix mypy issues --- changes/1192.bugfix.md | 2 +- hikari/impl/cache.py | 10 +++------- hikari/internal/cache.py | 3 --- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/changes/1192.bugfix.md b/changes/1192.bugfix.md index 93dcaedc01..f84e19a270 100644 --- a/changes/1192.bugfix.md +++ b/changes/1192.bugfix.md @@ -1,2 +1,2 @@ Properly garbage collect message references in the cache - - properly deserialize `PartialMessage.referenced_message` as the partial message it is + - Properly deserialize `PartialMessage.referenced_message` as a partial message diff --git a/hikari/impl/cache.py b/hikari/impl/cache.py index d2022f6273..6293788645 100644 --- a/hikari/impl/cache.py +++ b/hikari/impl/cache.py @@ -1564,15 +1564,11 @@ def _set_message( referenced_message: typing.Optional[cache_utility.RefCell[cache_utility.MessageData]] = None if message.referenced_message: reference_id = message.referenced_message.id + referenced_message = self._message_entries.get(reference_id) or self._referenced_messages.get(reference_id) - if referenced_message := ( - self._message_entries.get(reference_id) or self._referenced_messages.get(reference_id) - ): - # We just update the already cached message to prevent losing the link between referenced messages, - # which could lead to a memory leak when garbage collecting + if referenced_message: + # Since the message is partial, if we don't have it cached, there is nothing we can do about it referenced_message.object.update(message.referenced_message) - else: - referenced_message = self._set_message(message.referenced_message) # Only increment ref counts if this wasn't previously cached. if message.id not in self._referenced_messages and message.id not in self._message_entries: diff --git a/hikari/internal/cache.py b/hikari/internal/cache.py index 7f1bae284e..9980034731 100644 --- a/hikari/internal/cache.py +++ b/hikari/internal/cache.py @@ -723,9 +723,6 @@ def build_from_entity( if not member and message.member: member = RefCell(MemberData.build_from_entity(message.member)) - if not referenced_message and message.referenced_message: - referenced_message = RefCell(MessageData.build_from_entity(message.referenced_message)) - interaction = ( MessageInteractionData.build_from_entity(message.interaction, user=interaction_user) if message.interaction