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

connections graceful shutdown #13937

Closed
wants to merge 12 commits into from
Closed

Conversation

icing
Copy link
Contributor

@icing icing commented Jun 13, 2024

Implement graceful shutdown of connections in libcurl:

  • when a connection is ready to be discarded, run a non-blocking shutdown on all connection filters. If that returns errors, discard the connection. If that sucessfully shuts down all filters, discard as well. Otherwise, add it to a "shutdown" list in the transfers connection cache.
  • limit the shutdown list to the same maximum length as the connection cache's live limit. When shutdown down a new connection at the limit, the oldest one is discarded.
  • In multi_wait() and multi_waitfds(), collect all connection caches involved (each transfer might carry its own) into a temporary list. Let each connection cache on the list contribute sockets and POLLIN/OUT events it's connections are waiting for.
  • in multi_perform() collect the connection caches the same way and let them peform their maintenance. This will make another non-blocking attempt to shutdown all connections on its shutdown list.
  • when a connection cache is destroyed, discard all connections on the shutdown list. In debug builds, with the envionment variable CURL_GRACEFUL_SHUTDOWN, run a blocking shutdown (using poll) and the default 2 second timeout. This is for testing (see below) and might be exposed as an option to new libcurl API function.

TODO: multi_socket() does not know how to handle sockets from connection caches.

Details

  • similar to connected add a shutdown bit at the connection filter instance to remember it has already shutdown and need not be invoked again when we do repeated, non-blocking attempts to shut down the complete filter chain.
  • conncache structs have a tmp_list member that allows collecting caches onto a temporary list inside multi_wait and multi_perform handling. The list is always cleared before calls return.
  • replace Curl_discard(data, conn) with `Curl_conncache_shutdown_conn(data, conn,...) to handle keeping connections for non-blocking shutdown handling.
  • When CURL_DEBUG is in the environment and curl is a debug build, make the conncache closing handle verbose.
  • fix openssl shutdown to be way more careful about still receiving data after its side has shut down. HTTP/2 connections may have a server-side GOAWAY in transit. We need to cope with that without error.

Testing

  • add test cases using tcpdump to check if a curl run caused TCP RST packets. These checks are used in test_02_30, test_30_06 and test_31_06.

icing added 11 commits June 17, 2024 10:47
Implement graceful shutdown of connections in libcurl:
- when a connection is ready to be discarded, run a
  non-blocking shutdown on all connection filters.
  If that returns errors, discard the connection. If
  that sucessfully shuts down all filters, discard as well.
  Otherwise, add it to a "shutdown" list in the transfers
  connection cache.
- In multi_wait() and multi_waitfds(), collect all
  connection caches involved (each transfer might carry
  its own) into a temporary list. Let each connection
  cache on the list contribute sockets and POLLIN/OUT
  events it's connections are waiting for.
- in multi_perform() collect the connection caches the
  same way and let them peform their maintenance. This
  will make another non-blocking attempt to shutdown
  all connections on its shutdown list.
- TODO: multi_socket() does not know how to handle
  sockets from connection caches.
- when a connection cache is destroyed, discard all
  connections on the shutdown list. In debug builds,
  with the envionment variable CURL_GRACEFUL_SHUTDOWN,
  run a blocking shutdown (using poll) and the default
  2 second timeout. This is for testing (see below)
  and might be exposed as an option to new libcurl
  API function.

Details
- similar to `connected` add a `shutdown` bit at the connection
  filter instance to remember it has already shutdown and need
  not be invoked again when we do repeated, non-blocking attempts
  to shut down the complete filter chain.
- conncache structs have a `tmp_list` member that allows
  collecting caches onto a temporary list inside multi_wait
  and multi_perform handling. The list is always cleared before
  calls return.
- replace `Curl_discard(data, conn)` with
   `Curl_conncache_shutdown_conn(data, conn,...) to
   handle keeping connections for non-blocking
   shutdown handling.
- When CURL_DEBUG is in the environment and  curl is
  a debug build, make the conncache closing handle verbose.
- fix openssl shutdown to be way more careful about
  still receiving data after its side has shut down.
  HTTP/2 connections may have a server-side GOAWAY in
  transit. We need to cope with that without error.

Testing
- add test cases using `tcpdump` to check if a curl
  run caused TCP RST packets. These checks are used
  in test_02_30, test_30_06 and test_31_06.
test_02_3[01], check that we see tcp rst unless we ask
curl to shutdown gracefully.
Adapt call to tcpdump for linux.
- use the original easy handle - when available - to shutdown
  the handler parts of a connection. This is because protocol
  handlers like 'file:' rely on this for resource cleanup.
- should fix compiler warning on msys2 windows GHA.
- connection filter 'close' MUST no longer imply a 'shutdown'.
  Filter's close method now only close. Shutdown is called
  explicitly IFF the connection is in a clean state. Aborted
  connection are no longer shut down.
- implement behaviour in connection cache and vtls backends.
- adjust test1542 output since we now log different message
  on shutdown vs close
@icing
Copy link
Contributor Author

icing commented Jun 19, 2024

Closing to make an updated, squashed one.

@icing icing closed this Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

1 participant