Skip to content

Fix use of lwt_io + conduit-tls#328

Merged
dinosaure merged 6 commits into
masterfrom
fix-lwt-io
Oct 13, 2020
Merged

Fix use of lwt_io + conduit-tls#328
dinosaure merged 6 commits into
masterfrom
fix-lwt-io

Conversation

@dinosaure
Copy link
Copy Markdown
Member

According to the documentation of conduit-tls, the implementation with ocaml-tls is not thread-safe, something like both (send flow) (recv flow) is unsafe. It's mostly about the wrapping of a Conduit.flow with Lwt_io - because the flow can be a TLS flow, we must protect recv and send with a mutex anyway.

The question behind such patch is: should we protect conduit-tls with a mutex - and by this way, we are not mandatory to protect any flow with a mutex or should we keep this design? To this existential question, we should make an issue a talk about this issue later - note that this is not a part of the core of conduit.

The second patch is about the first-read behavior of Lwt_io. I'm not sure how we can properly schedule that but it seems that the Lwt_io wants to read first when we start a TLS connection - I suspect that it schedule the action from the underlying state of the socket and, because we receive a part of the handshake, it considers Conduit.recv as the first operation. However, for the HTTP/1.1 point-of-view, this is not true when the server expect to receive something - so client and server expect to receive something at the end.

conduit-tls is able to produce an opportunity to schedule Conduit.send even if we called Conduit.recv, it returns Input 0 (which means that we received nothing, but the connection still is established). On the other side, recv given to Lwt_io can yield to give the opportunity to the application to scheduler Conduit.send/Lwt_io.write.

Finally:

  1. the question about conduit-tls is a bit hard but an issue will be done which describe precisely the situation
  2. the problem with lwt_io is an inheritance of 1) and it seems that tls.lwt fix it in its own way

@dinosaure dinosaure mentioned this pull request Oct 12, 2020
3 tasks
@dinosaure
Copy link
Copy Markdown
Member Author

Ok, it seems that the first-recv behavior of Lwt_io is true in any case (TLS or not), the second commit gives the opportunity to reschedule Conduit.send with an Input 0.

@dinosaure
Copy link
Copy Markdown
Member Author

The real conclusion of this patch is: don't use Lwt_io when you need a full control of how to schedule recv/send...

@dinosaure dinosaure merged commit b51d6de into master Oct 13, 2020
@samoht samoht deleted the fix-lwt-io branch January 30, 2021 12:43
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.

1 participant