http4s / http4s-curl

curling isn't just for Canadians
Apache License 2.0
32 stars 8 forks source link

Implement a `CurlClient` based on `FileDescriptorPoller` #117

Open armanbilge opened 1 year ago

armanbilge commented 1 year ago

Currently to use http4s-curl we use CurlApp which replaces the IORuntime with a CurlRuntime. This means that it is not possible to integrate http4s-curl with FS2-based applications (Ember, Skunk, Lepus) and other native libraries (e.g. SNUnit).

In Cats Effect 3.6 the default runtime offers file descriptor I/O polling that works with all of these libraries. The idea would be to use IOApp as usual and then to construct a Resource[IO, Client[IO]] with a CurlClientBuilder (similar to Ember).

Here's a rough sketch for how to implement a Client[IO] using file descriptor polling:

  1. Create a MapRef[IO, FileDescriptor, (Fiber, Fiber)] to keep track of a dedicated fiber for polling each relevant file descriptor for each type of interest (read interest vs write interest).

  2. Configure the CURLMOPT_SOCKETFUNCTION to get notified about when cURL needs to start/stop monitoring sockets for read/writes. Use these callbacks to start/cancel fibers in the MapRef. https://curl.se/libcurl/c/CURLMOPT_SOCKETFUNCTION.html

  3. To add a new socket you should register the file descriptor.

  4. Then, start a fiber looping on read-readiness and another looping on write-readiness. Whenever the socket is ready, the curl_multi_socket_action should be invoked. https://curl.se/libcurl/c/curl_multi_socket_action.html

  5. A callback for CURLMOPT_TIMERFUNCTION should also be registered. It should start a fiber that sleeps the requested time and then calls curl_multi_socket_action. https://curl.se/libcurl/c/CURLMOPT_TIMERFUNCTION.html

hnaderi commented 1 year ago

@armanbilge Interesting! :heart_eyes: Thanks for providing all the details :relaxed: I'm currently overwhelmed by the sheer amount of work I have to do in my job, but I'll take care of this one as soon as I can.

hnaderi commented 1 year ago

I read the documents and implemented some stuff required for supporting curl multi-socket which is the base of this. I've got a couple of questions:

  1. In this section:

    Then, start a fiber looping on read-readiness and another looping on write-readiness. Whenever the socket is ready, the curl_multi_socket_action should be invoked.

I can't understand how this looping needs to be implemented. Libcurl requires to be notified when there are some changes on the FD state, but this API seems to be for detecting the changes themselves by checking the FD? Am I correct or I'm missing something?

Whenever the socket is ready

Or to put it in other words, how to detect if the socket is ready?

  1. How to get a FileDescriptorPoller? Are there constructors somewhere? Currently I just ignored this by getting it in the constructor arguments, but I wanted to know the big picture and how it's going to be in the final builder?
armanbilge commented 1 year ago

I can't understand how this looping needs to be implemented.

pollReadRec and pollWriteRec already implement that loop :) those methods will call the provided function every time the socket becomes ready on reading or writing.

So basically you will want to start a fiber like this:

pollHandle.pollReadRec(()) { _ =>
  IO {
    curl_multi_socket_action(multi_handle, sockfd, CURL_CSELECT_IN, null)
    Left(()) // loop forever
  }
}

and a similar one for the write loop. This is a fiber that will invoke curl_multi_socket_action with the appropriate arguments every time the socket indicates readiness for read/write.


2. How to get a FileDescriptorPoller?

This is the most unstable part of the API, that Daniel and I are still debating over. Here's how to do it currently, but it might change.

https://github.com/typelevel/fs2/blob/40241c8e803a1730fe1390a09dc5ad2504c883fc/io/native/src/main/scala/fs2/io/ioplatform.scala#L45-L52

hnaderi commented 1 year ago

Got it, Thank you!

armanbilge commented 1 year ago

Thanks for your work, don't hesitate if you have other questions!! 🚀

hnaderi commented 1 year ago

The tests are passing for the new runtime, however it needs polishing and some cleaning. And also one side effect of using this new runtime is that it doesn't support windows. @armanbilge What do you think about this? I think we can keep both the old runtime and this new implementation using FD poller, in order not to drop the win support.

armanbilge commented 1 year ago

The tests are passing for the new runtime

Wow, amazing!! Thanks so much for trying this out.

I think we can keep both the old runtime and this new implementation using FD poller, in order not to drop the win support.

Yes, this is exactly what I'm thinking as well, if it's not too much maintenance burden we should keep both implementations.

Edit:

both the old runtime

Btw, the old runtime can be refactored as a PollingSystem.

hnaderi commented 1 year ago

Btw, the old runtime can be refactored as a PollingSystem.

Yes, I'm gonna do it after cleaning this one. Currently I've just put a nowarn annotation on it so that I can focus on the task at hand.