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

multi-socket support #124

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

multi-socket support #124

wants to merge 13 commits into from

Conversation

hnaderi
Copy link
Collaborator

@hnaderi hnaderi commented Jun 29, 2023

closes #117

@hnaderi hnaderi force-pushed the multi-socket-support branch 3 times, most recently from c73a986 to 5519733 Compare July 4, 2023 10:56
This implementation uses curl multi socket drive, and uses
FileDescriptorPoller from cats effect.
It allows using this library alongside other cats effect libraries.

override def needsPoll(poller: Poller): Boolean = needsPoll

override def interrupt(targetThread: Thread, targetPoller: Poller): Unit = ()
Copy link
Member

Choose a reason for hiding this comment

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

This is fine for now because everything is single-threaded, but to support multi-threading in the future we will have to figure out a way to interrupt curl_multi_poll somehow 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that was my concern as well. I hope we can use the proper event-driven approach for Windows by then, so we deprecate this polling stuff completely 😄

Copy link
Member

Choose a reason for hiding this comment

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

but to support multi-threading in the future we will have to figure out a way to interrupt curl_multi_poll somehow

Aha :)

https://curl.se/libcurl/c/curl_multi_wakeup.html

@hnaderi hnaderi marked this pull request as ready for review July 5, 2023 09:52
@hnaderi hnaderi requested a review from armanbilge July 5, 2023 09:55
@hnaderi
Copy link
Collaborator Author

hnaderi commented Jul 9, 2023

Apparently there is some race condition in this implementation, as multi socket runtime tests don't terminate sometimes. Sometimes even the test timeout does not work, that might give a clue for next investigations.
I also updated the tests to use more parallelism to increase the likeliness of this problem, so we can find it more easily.

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.

Implement a CurlClient based on FileDescriptorPoller
2 participants