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 thread-safety issues in TCPThroughputBenchmark #2537

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Oct 9, 2023

Motivation:

Several thread-safety issues were missed in code review. This patch fixes them.

Modifications:

  • Removed the use of an unstructured Task, replaced with eventLoop.execute to ServerHandler's EventLoop.
  • Stopped ClientHandler reaching into the benchmark object without any synchronization, used promises and event loop hops instead.

Result:

Thread safety is back

@Lukasa Lukasa added the semver/none No version bump required. label Oct 9, 2023
Motivation:

Several thread-safety issues were missed in code review. This patch fixes them.

Modifications:

- Removed the use of an unstructured Task, replaced with eventLoop.execute to
  ServerHandler's EventLoop.
- Stopped ClientHandler reaching into the benchmark object without any
  synchronization, used promises and event loop hops instead.

Result:

Thread safety is back
@Lukasa Lukasa force-pushed the cb-fix-thread-safety-issues-in-tests branch from 7bc4f67 to 7fc23a5 Compare October 9, 2023 09:06
@FranzBusch FranzBusch enabled auto-merge (squash) October 25, 2023 08:47
@FranzBusch FranzBusch merged commit 54c85cb into apple:main Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants