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

Remove use of thread locks in crt bindings #436

Merged
merged 3 commits into from
Mar 13, 2025

Conversation

JordonPhillips
Copy link
Contributor

@JordonPhillips JordonPhillips commented Mar 12, 2025

This primarily serves the purpose of removing thread-based locks from the CRT bindings, but it also ensures that the status code and headers have been received before returning from send.

Thread-based Locks will deadlock if acquired twice by the same thread, so it's likely to happen in async code that runs multiple coroutines in one thread. And also, deques are already thread-safe so there's no need to protect them further.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@JordonPhillips JordonPhillips requested a review from a team as a code owner March 12, 2025 22:37
This primarily serves the purpose of removing thread-based locks
from the CRT bindings, but it also ensures that the status code
and headers have been received before returning from send.
@JordonPhillips JordonPhillips force-pushed the await-crt-initial-http branch from 28f7884 to 16c5e4e Compare March 13, 2025 11:52
@JordonPhillips JordonPhillips changed the title Remove use of thread futures in crt bindings Remove use of thread locks in crt bindings Mar 13, 2025
alextwoods
alextwoods previously approved these changes Mar 13, 2025
self, status_code: int, headers: list[tuple[str, str]], **kwargs: Any
) -> None: # pragma: crt-callback
print("on-response")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: remove.

@nateprewitt nateprewitt merged commit bd17d3e into develop Mar 13, 2025
2 checks passed
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

Successfully merging this pull request may close these issues.

3 participants