Lwt_io.establish_server (TCP servers): expose client socket to connection-handling callback#586
Merged
Conversation
This commit consists of whitespace edits and alpha-renaming.
- Avoid Lwt.wakeup (synchronous control flow transfer). Use Lwt.wakeup_later. - Use Lwt.async instead of Lwt.ignore_result. This has the side effect that exceptions during server startup (bind, listen) now go into rejecting the promise that Lwt_io.establish_server returns. Before, they were raised up the stack. This behavior was not specified, however. Most users can be expected to be ready for both behaviors, and the old behavior was a bug.
aantron
force-pushed
the
establish-server-fd
branch
from
56c1b6e to
e658860
Compare
Collaborator
Author
|
This was motivated by inhabitedtype/httpaf#54. inhabitedtype/httpaf#53 also anticipated the type signature of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a follow-on to #323/#346 (cc @rgrinberg). It adds a function like:
...whereas existing versions of this function didn't pass the
Lwt_unix.file_descrto the client-handling function, and only passed thesockaddrand the two channels.I'm still not sure if this is the right API, but starting with this.
The PR also includes a pretty thorough cleanup of the existing code, so it should be easier to keep working on.