Skip to content
This repository was archived by the owner on Feb 27, 2025. It is now read-only.

[TECH_DEBT] - Make it harder to leak an active event receiver #3802

Open
jbearer opened this issue Oct 25, 2024 · 0 comments
Open

[TECH_DEBT] - Make it harder to leak an active event receiver #3802

jbearer opened this issue Oct 25, 2024 · 0 comments

Comments

@jbearer
Copy link
Member

jbearer commented Oct 25, 2024

What is this task and why do we need to work on it?

Active event receivers are dangerous because they must be consumed from, or they will accumulate a backlog of events, which causes all sorts of issues, including:

  • memory leaks
  • channel overflows
  • processing really old events when we finally do get through the queue => causes weird behavior

Right now, it is really easy to accidentally create an active receiver that we never actually read from, for example when creating a receiver for the sole purpose of cloning it and giving it to subtasks: this should be an inactive receiver but there is nothing that will stop you from making it active.

What work will need to be done to complete this task?

Some ideas:

  • Active receivers should not be clonable. The one and only thing you should be able to do with them is read from them, and if you don't do that clippy should warn about something being unused. We can wrap Receiver in a newtype that doesn't implement clone, thus if you want a clonable receiver you are forced to make it inactive and only activate it when you are ready to use it
  • Instead of dealing directly with receivers, we can have a wrapper function that just returns a #[must_use] stream of events, or a function that takes an inactive receiver and a handler, activates the receiver and then eagerly consumes events and passes them to the handler

Are there any other details to include?

No response

What are the acceptance criteria to close this issue?

  • There are no more cases in the code where we take an active receiver and don't consume events from it
  • There is some kind of warning or error if we did do that

Branch work will be merged to (if not the default branch)

No response

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

No branches or pull requests

1 participant