Skip to content

Commit

Permalink
Automatically copy the cached dict result from generate_packet_map
Browse files Browse the repository at this point in the history
Before this change, if the user called `generate_packet_map`, a dict
object would get cached, and that same object (reference) would be
actually returned. This means that when the fucntion would get called
again, the same cached dict would be returned.

Generally, this wouldn't be an issue, however if the user modifies this
returned dictionary, since caching only stores the very same reference,
on next call the modified version of that dict would be returned again.

This was mentioned in the function's docstring as a warning, however I
don't see that as sufficient, as people will usually do stupid things
with libraries, without reading the docs, which in this case, would
potentially end up breaking a lot of things.

To avoid this issue, another decorrator was created, responsible for
copying the returned object after the cache decorator. That means the
cached version will hold a dict that no user will be able to modify
accidentally, as the returned dict will simply be a copy of it.

This does mean using slightly more memory, but it's a pretty small
amount, and it's well worth the potential mess-ups this prevents.
  • Loading branch information
ItsDrike committed Jun 11, 2023
1 parent 40ae446 commit af54ac1
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
8 changes: 3 additions & 5 deletions mcproto/packets/packet_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from typing import Literal, NamedTuple, NoReturn, overload

from mcproto.packets.packet import ClientBoundPacket, GameState, Packet, PacketDirection, ServerBoundPacket
from mcproto.utils.decorators import copied_return

__all__ = ["generate_packet_map"]

Expand Down Expand Up @@ -94,8 +95,9 @@ def generate_packet_map(
...


@copied_return
@lru_cache()
def generate_packet_map(direction: PacketDirection, state: GameState) -> Mapping[int, type[Packet]]:
def generate_packet_map(direction: PacketDirection, state: GameState) -> dict[int, type[Packet]]:
"""Dynamically generated a packet map for given ``direction`` and ``state``.
This generation is done by dynamically importing all of the modules containing these packets,
Expand All @@ -105,10 +107,6 @@ def generate_packet_map(direction: PacketDirection, state: GameState) -> Mapping
As this fucntion is likely to be called quite often, and it uses dynamic importing to obtain
the packet classes, this function is cached, which means the logic only actually runs once,
after which, for the same arguments, the same dict will be returned.
..warning: As this function is cached, make sure to avoid modifying the returned dictionary,
as that will directly modify the stored version in the cache, leading to future calls
with the same arguments returning a wrong (modified) version of this dictionary.
"""
module = importlib.import_module(MODULE_PATHS[state])

Expand Down
29 changes: 29 additions & 0 deletions mcproto/utils/decorators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
from __future__ import annotations

from collections.abc import Callable
from functools import wraps
from typing import Protocol, TypeVar

from typing_extensions import ParamSpec

__all__ = ["copied_return"]

T = TypeVar("T")


class SupportsCopy(Protocol):
def copy(self: T) -> T:
...


P = ParamSpec("P")
R_Copyable = TypeVar("R_Copyable", bound=SupportsCopy)


def copied_return(func: Callable[P, R_Copyable]) -> Callable[P, R_Copyable]:
@wraps(func)
def wrapper(*args: P.args, **kwargs: P.kwargs) -> R_Copyable:
ret = func(*args, **kwargs)
return ret.copy()

return wrapper

0 comments on commit af54ac1

Please sign in to comment.