Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Events With Multiple Local Handlers Can Be Interfered By Each Other #16

Open
melvinkcx opened this issue Dec 6, 2021 · 3 comments
Open

Comments

@melvinkcx
Copy link
Owner

Today, an event with multiple local handlers registered can interfere with each other as all handlers are given the same shared copy of events.

For instance,

# anywhere in code

dispatch(Events.USER_CREATED, {"key_in_payload": 123})
# in handlers.py

@local_handler.register(event_name=Events.USER_CREATED)
async def handle_user_created(event: Event):
    _, payload = event
    payload.pop("key_in_payload")


@local_handler.register(event_name=Events.USER_CREATED)
async def handle_user_created_2(event: Event)
    _, payload = event
    payload["key_in_payload"]  # KeyError

Proposal

A copy of the payload should be passed to the handlers.

@danielhasan1
Copy link

The proposal will solve this issue on a small level, but when the payload size is big and there are multiple consumers who are consuming the dispatched event at the same time this will become a big issue it will block all the memory. Especially for long-running tasks or coroutines.
A quick fix that comes into my mind is writing a wrapper inside dispatch to make the payload a frozen dict.
multiple handlers could accept it, frozen dict won't change. if anyone wants to change any property of that dict they could create a separate copy of that dict or get items from that dict.

@melvinkcx
Copy link
Owner Author

@danielhasan1 you're right. I haven't tried to address this issue because the payload can technically be of any type, not just dict. Either approach risks breaking the user's code if the payload contains values (custom objects, etc) that cannot be cloned or frozen.

I'm also not aware of any "standard" approach to making a dict immutable as PEP 416 was rejected and PEP 603 is still under drafting stage.

I think it's best we keep this unfixed for now.

@ebrahimradi
Copy link

I think it's not about this library, it's about our design. events should be immutable. as a developer you should try design to have immutable events. for example you can try pydantic frozen classes or frozen dataclass or something that guarantee immutable events.
one suggestion is to change library to only accept immutable events data :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants