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

close descriptor when failed to open connection #97

Merged
merged 1 commit into from
May 28, 2021

Conversation

EduardoRFS
Copy link
Contributor

Currently if the connection failed to start the file descriptor is still open, which blows up lwt when using the select engine as soon as it hits 1024 FDs opens.

Example

List.init(10_000, _ => ())
|> Lwt_list.iter_s(() => {
     let uri = Uri.of_string("http://localhost:1234");
     Piaf.Client.Oneshot.get(uri) |> Lwt.map(_ => ());
   })
|> Lwt_main.run;

@@ -166,14 +166,19 @@ let make_impl ~config ~conn_info fd =
let open Lwt_result.Syntax in
let* () = connect ~config ~conn_info fd in
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TLDR: If this line failed, that would lead to leak of descriptor

@anmonteiro
Copy link
Owner

Tests are blocked by #95 which will fix them.

@anmonteiro anmonteiro force-pushed the fix-infinite-connections branch from b1e0a8c to afeacad Compare May 28, 2021 22:22
@anmonteiro
Copy link
Owner

Thanks!

@anmonteiro anmonteiro merged commit fa65c14 into anmonteiro:master May 28, 2021
anmonteiro added a commit that referenced this pull request May 28, 2021
@EduardoRFS EduardoRFS deleted the fix-infinite-connections branch May 28, 2021 23:10
anmonteiro added a commit to anmonteiro/opam-repository that referenced this pull request Sep 6, 2024
CHANGES:

- Improve certificate checking and authentication
  ([anmonteiro/piaf#93](anmonteiro/piaf#93)) -
  [@Firgeis](https://github.com/Firgeis)
- Check certificate SAN IP address when appropriate
  ([anmonteiro/piaf#96](anmonteiro/piaf#96)) -
  [@Firgeis](https://github.com/Firgeis)
- Close the file descriptor when failing to open a connection
  ([anmonteiro/piaf#97](anmonteiro/piaf#97)) -
  [@EduardoRFS](https://github.com/EduardoRFS)
- Yield to other threads when reading a message body. This improves fairness
  for large message bodies
  ([anmonteiro/piaf#100](anmonteiro/piaf#100))
- Add error handling to `Response.of_file`
  ([anmonteiro/piaf#103](anmonteiro/piaf#103))
- Add `Client.send` which sends a `Request.t`
  ([anmonteiro/piaf#110](anmonteiro/piaf#110))
- openssl: set the client verify callback
  ([anmonteiro/piaf#112](anmonteiro/piaf#112))
- Piaf.Response: add `or_internal_error`
  ([anmonteiro/piaf#120](anmonteiro/piaf#120))
- Piaf.Response: Add `Body.sendfile` and `Response.sendfile`
  ([anmonteiro/piaf#124](anmonteiro/piaf#124))
- Piaf.Config: Add `config.flush_headers_immediately`
  ([anmonteiro/piaf#125](anmonteiro/piaf#125))
- Piaf.Server: Add `config.shutdown_timeout` to wait before shutting down the
  Piaf server ([anmonteiro/piaf#174](anmonteiro/piaf#174))
- Websocket support ([anmonteiro/piaf#139](anmonteiro/piaf#139))
- Multicore support ([anmonteiro/piaf#151](anmonteiro/piaf#151))
- Allow binding to UNIX domain socket
  ([anmonteiro/piaf#161](anmonteiro/piaf#161))
- Don't send invalid HTTP/2 headers
  ([anmonteiro/piaf#197](anmonteiro/piaf#197))
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.

2 participants