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

Hold a per Client state rather than a static state. #317

Closed
Totodore opened this issue May 6, 2024 · 1 comment · Fixed by #330
Closed

Hold a per Client state rather than a static state. #317

Totodore opened this issue May 6, 2024 · 1 comment · Fixed by #330
Labels
bug Something isn't working help wanted Extra attention is needed refactoring This reference a need for a refactoring

Comments

@Totodore
Copy link
Owner

Totodore commented May 6, 2024

Currently the global state management is done with a static state. It is convenient because the user can put non clonable data and get a 'static ref from any callback. However this should be completely avoided for the following reasons:

  • With multiple clients spawned, they are going to share the same state and this will probably lead to bugs. First example with all the integration tests for the socketioxide lib.
  • Currently the implementation uses unsafe code to do this. Moving to a per client clonable state remove this.
  • This will lead to a possible axum-like state implementation. Having only one typed state rather than the type erased typemap.
@Totodore Totodore added bug Something isn't working refactoring This reference a need for a refactoring labels May 6, 2024
@Totodore Totodore assigned Totodore and unassigned Totodore May 6, 2024
@Totodore Totodore added the help wanted Extra attention is needed label May 8, 2024
@Totodore
Copy link
Owner Author

Totodore commented May 8, 2024

If someone want to contribute for this issue:

  • Move the state typemap to the Client struct and add a clone bound.
  • Pass the typemap to each handler.
  • Clone the extracted state from the typemap in the extractor.

@Totodore Totodore linked a pull request Jun 5, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed refactoring This reference a need for a refactoring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant