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

fix: fix server multithreading #5

Merged
merged 2 commits into from
Dec 17, 2023
Merged

fix: fix server multithreading #5

merged 2 commits into from
Dec 17, 2023

Conversation

edwinkys
Copy link
Member

@edwinkys edwinkys commented Dec 16, 2023

Purpose

This PR solves the issue where the server doesn't acccept multiple connections/requests at once. It will throw connection refused error or even sometimes crash the server.

Approach

  1. I modified the server in a way to prevent direct mutation to the server by using Arc and Mutex for shared values.
  2. I fixed the tokio::spawn code which is the primary source of this problem.
  3. I remove the Server.serve() function and replace it with a standalone serve function that will handle the server and thread creation. In this function, the server is wrapped by Arc to allow threads to refer to the same server.

Testing

  • I have tested this PR locally.
  • I added tests to cover my changes, if not applicable, I have added a reason why.

I have modified the test suite to accomodate the changes that I made to the server functionality.

I tested this PR by using a custom Python script to create a multi-threaded request to the server while the server is running on localhost port 3141. Without this changes, the server will throw an error when encountering multi-thread requests.

Chore checklist

  • I have updated the documentation accordingly.
  • I have added comments to most of my code, particularly in hard-to-understand areas.

@edwinkys edwinkys added the enhancement New feature or request label Dec 16, 2023
@edwinkys edwinkys self-assigned this Dec 16, 2023
@edwinkys edwinkys added the bug Something isn't working label Dec 17, 2023
@edwinkys edwinkys merged commit d1add95 into main Dec 17, 2023
@edwinkys edwinkys deleted the fix-server-multithreading branch December 17, 2023 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant