Issue is reproducible for me if I have made request with multi handle,
then I make request that will take very long and then I make request
that should be fast again, however what happens it is that it seems
to think that timeout was not changed and it makes it not call initial
`CURLMOPT_TIMERFUNCTION`.
Closes#15627
Introduce five functions named after the state they serve:
- state_connect for MSTATE_CONNECT
- state_do for MSTATE_DO
- state_performing for MSTATE_PERFORMING
- state_ratelimiting for MSTATE_RATELIMITING
- state_resolving for MSTATE_RESOLVING
Closes#15418
It makes the callbacks get different signnatures when used from within
libcurl vs outside of it by libcurl-using applications (such as the
libtests) and this triggers UndefinedBehaviorSanitizer errors.
Closes#15289
When a multi handle is being cleaned up, it can still cause user
callbacks to be fired. Notably Curl_cpool_destroy calls socket_callback
on all pooled connections. It's still possible for the callback to call
curl_multi_assign leading to an assert.
This commit moves clearing of a multi handle magic to a point where the
multi handle stops being a valid object.
Fixes#15201Closes#15206
booleans should use the type 'bool' and set the value to TRUE/FALSE
non-booleans should not be 'bool' and should not set the value to
TRUE/FALSE
Closes#15123
For all states before MSTATE_DO the connect timeout needs to be
considered.
Regression since #13371 (be659030ba) shipped in 8.8.0
Reported-by: Deniz Sökmen
Fixes#15100Closes#15119
The proper alignment is not guaranteed. This function now instead uses
only the first and last byte of the key since they are the ones likely
to change most (one of them, depending on CPU endian) and the hash is
tiny anyway.
Closes#15063
Warn/assert about a possibly stalling transfer only when it
has no timeout set.
The assertion triggered in test 1540 on loaded CI sometimes.
Closes#15028
With higher parallelism in CI, the ASSERT triggered on pause tests.
Strengthen the check. We might want to think about removing
KEEP_RECV_PAUSE|KEEP_SEND_PAUSE altogether.
Closes#14981
By requiring that the multi handle is fine, it can detect bad usage
better and by that avoid crashes. Like in the #14860 case, which is an
application calling curl_multi_assign() with a NULL pointer multi
handle.
Reported-by: Carlo Cabrera
Fixes#14860Closes#14862
This is a better match for what they do and the general "cpool"
var/function prefix works well.
The pool now handles very long hostnames correctly.
The following changes have been made:
* 'struct connectdata', e.g. connections, keep new members
named `destination` and ' destination_len' that fully specifies
interface+port+hostname of where the connection is going to.
This is used in the pool for "bundling" of connections with
the same destination. There is no limit on the length any more.
* Locking: all locks are done inside conncache.c when calling
into the pool and released on return. This eliminates hazards
of the callers keeping track.
* 'struct connectbundle' is now internal to the pool. It is no
longer referenced by a connection.
* 'bundle->multiuse' no longer exists. HTTP/2 and 3 and TLS filters
no longer need to set it. Instead, the multi checks on leaving
MSTATE_CONNECT or MSTATE_CONNECTING if the connection is now
multiplexed and new, e.g. not conn->bits.reuse. In that case
the processing of pending handles is triggered.
* The pool's init is provided with a callback to invoke on all
connections being discarded. This allows the cleanups in
`Curl_disconnect` to run, wherever it is decided to retire
a connection.
* Several pool operations can now be fully done with one call.
Pruning dead connections, upkeep and checks on pool limits
can now directly discard connections and need no longer return
those to the caller for doing that (as we have now the callback
described above).
* Finding a connection for reuse is now done via `Curl_cpool_find()`
and the caller provides callbacks to evaluate the connection
candidates.
* The 'Curl_cpool_check_limits()' now directly uses the max values
that may be set in the transfer's multi. No need to pass them
around. Curl_multi_max_host_connections() and
Curl_multi_max_total_connections() are gone.
* Add method 'Curl_node_llist()' to get the llist a node is in.
Used in cpool to verify connection are indeed in the list (or
not in any list) as they need to.
I left the conncache.[ch] as is for now and also did not touch the
documentation. If we update that outside the feature window, we can
do this in a separate PR.
Multi-thread safety is not achieved by this PR, but since more details
on how pools operate are now "internal" it is a better starting
point to go for this in the future.
Closes#14662
- Renames Curl_readwrite() to Curl_sendrecv() to reflect that it
is mainly about talking to the server, not reads or writes to the
client. Add a `nowp` parameter since the single caller already
has this.
- Curl_sendrecv() now runs all possible operations whenever it is
called and either it had been polling sockets or the 'select_bits'
are set.
POLL_IN/POLL_OUT are not always directly related to send/recv
operations. Filters like HTTP/2, QUIC or TLS may monitor reverse
directions. If a transfer does not want to send (KEEP_SEND), it
will not do so, as before. Same for receives.
- Curl_update_timer() now checks the absolute timestamp of an expiry
and the last/new timeout to determine if the application needs
to stop/start/restart its timer. This fixes edge cases where
updates did not happen as they should have.
- improved --test-event curl_easy_perform() simulation to handle
situations where no sockets are registered but a timeout is
in place.
- fixed bug in events_socket() that complained about removing
a socket that was unknown, when indeed it had removed the socket
just before, only it was the last in the list
- fixed conncache's internal handle to carry the multi instance
(where the cache has one) so that operations on the closure handle
trigger event callbacks correctly.
- fixed conncache to not POLL_REMOVE a socket twice when a conneciton
was closed.
Closes#14561
- add set/get functions for the custom data in a tree node
- use Curl_timediff for time comparisons instead of the custom macro, as they
do the exact same things.
- add asserts to catch mistakes better
- updated test 1309 accordingly
Closes#14562
Previously an error from the callback accidentally made libcurl keep the
hash entry which would lead to the entry remaining and then potentially
getting removed *again* which would lead to internal confusions.
This is an old issue (introduced in 2b3dd01b77), caught by the new
asserts from c0233a35da.
Closes#14557
- converted the Curl_hash_count() macro to a function
- Discourage accessing struct fields directly
- Document the internal API in HASH.md
Closes#14503
`data->id` is unique in *most* situations, but not in all. If a libcurl
application uses more than one connection cache, they will overlap. This
is a rare situations, but libcurl apps do crazy things. However, for
informative things, like tracing, `data->id` is superior, since it
assigns new ids in curl's serial curl_easy_perform() use.
Introduce `data->mid` which is a unique identifer inside one multi
instance, assigned on multi_add_handle() and cleared on
multi_remove_handle().
Use the `mid` in DoH operations and also in h2/h3 stream hashes.
Reported-by: 罗朝辉
Fixes#14414Closes#14499
- Turned them all into functions to also do asserts etc.
- The llist related structs got all their fields renamed in order to make
sure no existing code remains using direct access.
- Each list node struct now points back to the list it "lives in", so
Curl_node_remove() no longer needs the list pointer.
- Rename the node struct and some of the access functions.
- Added lots of ASSERTs to verify API being used correctly
- Fix some cases of API misuse
Add docs/LLIST.md documenting the internal linked list API.
Closes#14485
Instead of having an especially "unique" linked list handler for the
main list of easy handles within the multi handle, this now uses a
regular Curl_llist for this as well.
With this change, it is also clearer that every easy handle added to a
multi handle belongs to one and only one out of three different lists:
process - the general one for normal transfer processing
pending - queued up waiting to get a connection (MSTATE_PENDING)
msgsent - transfer completed (MSTATE_MSGSENT)
An easy handle must therefore be removed from the current list before it
gets added to another.
Closes#14474
Use these words and casing more consistently across text, comments and
one curl tool output:
AIX, ALPN, ANSI, BSD, Cygwin, Darwin, FreeBSD, GitHub, HP-UX, Linux,
macOS, MS-DOS, MSYS, MinGW, NTLM, POSIX, Solaris, UNIX, Unix, Unicode,
WINE, WebDAV, Win32, winbind, WinIDN, Windows, Windows CE, Winsock.
Mostly OS names and a few more.
Also a couple of other minor text fixups.
Closes#14360
Since data can be held in connection filter buffers when sending gives
EAGAIN, add methods to query this and perform flushing of those buffers.
The transfer loop will continue sending until all upload data is
processed and the connection is flushed.
- add `CF_QUERY_SEND_PENDING` to query filters
- add `CF_CTRL_DATA_SEND_FLUSH` to flush filters
- change `Curl_req_want_send()` to query the connection
if it needs flushing
- use `Curl_req_want_send()` to determine the POLLOUT
in the PERFORMING multi state
- implement flush handling in the HTTP/2 connection filter
Closes#14271
Replace Curl_resolv_unlock() with Curl_resolv_unlink():
-replace inuse member with refcount in Curl_dns_entry
- pass Curl_dns_entry ** to unlink, so it gets always cleared
- solve potential (but unlikley) UAF in FTP's handling of looked up
Curl_dns_entry. Esp. do not use addr information after unlinking an entry.
In reality, the unlink will not free memory, as the dns entry is still
referenced by the hostcache. But this is not safe and relying on no other
code pruning the cache in the meantime.
- pass permanent flag when adding a dns entry instead of fixing timestamp
afterwards.
url.c: fold several static *resolve_* functions into one.
Closes#14195
- multi.c: when ratelimiting a transfer stops (MSTATE_RATELIMITING ->
MSTATE_PERFORMING), run the MSTATE_PERFORMING state right away
- urldata.h: factor out upload and download progress counters into a
struct, use that for passing these to progress update functions
- progress.c/getinfo.c: change names of moved progress counters
- progress.c: use new structs and a helper struct to factor repeated
calculation into static helpers
Closes#14335
connections being shutdown would register sockets for events, but then
never remove these sockets again. Nor would the shutdown effectively
been performed.
- If a socket event involves a transfer, check if that is the
connection cache internal handle and run its multi_perform()
instead (the internal handle is used for all shutdowns).
- When a timer triggers for a transfer, check also if it is
about the connection cache internal handle.
- During processing shutdowns in the connection cache, assess
the shutdown timeouts. Register a Curl_expire() of the lowest
value for the cache's internal handle.
Reported-by: Gordon Parke
Fixes#14280Closes#14296
The input string pointer to:
curl_escape
curl_easy_escape
curl_unescape
curl_easy_unescape
The running_handles pointer to:
curl_multi_perform
curl_multi_socket_action
curl_multi_socket_all
curl_multi_socket
Reported-by: icy17 on github
Fixes#14247Closes#14262
- implement the socket hash user/reader/writer processing also
for connections that are being shut down by the connection cache.
- split out handling of current vs. last pollset socket event handling
into a function available in other code parts
- add `shutdown_poll` pollset to `connectdata` struct so that changes
in the pollset can be recorded during shutdown. (The internal handle
cannot keep it since it might be used for many connections)
Reported-by: calvin2021y on github
Fixes#14252Closes#14257
When removing an easy handle that had DoH sub-easy handles going, those
were not removed from the multi handle. Their memory was reclaimed on
curl_easy_cleanup() of the owning handle, but multi still had them in
their list.
Add `Curl_doh_close()` and `Curl_doh_cleanup()` as common point for
handling the DoH resource management. Use the `multi` present in the doh
handles (if so), for removal, as the `data->multi` might already have
been NULLed at this time.
Reported-by: 罗朝辉
Fixes#14207Closes#14212
Give warning for an empty pollset only when the connection has at least
IP connectivity. There are cases where the connect in QUIC makes another
attempt on a timeout and no socket will be available during that.
Closes#14108
- add a DEBUGASSERT for when a transfer's pollset should not be empty.
- move write unpausing from transfer loop into curl_easy_pause. This
make sure that the url_updatesocket() finds the correct state when
updating socket events.
- fix HTTP/2 proxy during connect phase to set sockets correctly
- fix test2600 to simulate a socket set
- move write unpausing from transfer loop into curl_easy_pause. This
make sure that the url_updatesocket() finds the correct state when
updating socket events.
- waiting for the resolver to deliver might not involve any sockets to
wait for. Do not generate a warning.
Fixes#14047Closes#14074
Based on the standards and guidelines we use for our documentation.
- expand contractions (they're => they are etc)
- host name = > hostname
- file name => filename
- user name = username
- man page => manpage
- run-time => runtime
- set-up => setup
- back-end => backend
- a HTTP => an HTTP
- Two spaces after a period => one space after period
Closes#14073
When libcurl discards a connection there are two phases this may go
through: "shutdown" and "closing". If a connection is aborted, the
shutdown phase is skipped and it is closed right away.
The connection filters attached to the connection implement the phases
in their `do_shutdown()` and `do_close()` callbacks. Filters carry now a
`shutdown` flags next to `connected` to keep track of the shutdown
operation.
Filters are shut down from top to bottom. If a filter is not connected,
its shutdown is skipped. Notable filters that *do* something during
shutdown are HTTP/2 and TLS. HTTP/2 sends the GOAWAY frame. TLS sends
its close notify and expects to receive a close notify from the server.
As sends and receives may EAGAIN on the network, a shutdown is often not
successful right away and needs to poll the connection's socket(s). To
facilitate this, such connections are placed on a new shutdown list
inside the connection cache.
Since managing this list requires the cooperation of a multi handle,
only the connection cache belonging to a multi handle is used. If a
connection was in another cache when being discarded, it is removed
there and added to the multi's cache. If no multi handle is available at
that time, the connection is shutdown and closed in a one-time,
best-effort attempt.
When a multi handle is destroyed, all connection still on the shutdown
list are discarded with a final shutdown attempt and close. In curl
debug builds, the environment variable `CURL_GRACEFUL_SHUTDOWN` can be
set to make this graceful with a timeout in milliseconds given by the
variable.
The shutdown list is limited to the max number of connections configured
for a multi cache. Set via CURLMOPT_MAX_TOTAL_CONNECTIONS. When the
limit is reached, the oldest connection on the shutdown list 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.
- for event based multis (multi->socket_cb set), add the sockets and
their poll events via the callback. When `multi_socket()` is invoked
for a socket not known by an active transfer, forward this to the
multi's cache for processing. On closing a connection, remove its
socket(s) via the callback.
TLS connection filters MUST NOT send close nofity messages in their
`do_close()` implementation. The reason is that a TLS close notify
signals a success. When a connection is aborted and skips its shutdown
phase, the server needs to see a missing close notify to detect
something has gone wrong.
A graceful shutdown of FTP's data connection is performed implicitly
before regarding the upload/download as complete and continuing on the
control connection. For FTP without TLS, there is just the socket close
happening. But with TLS, the sent/received close notify signals that the
transfer is complete and healthy. Servers like `vsftpd` verify that and
reject uploads without a TLS close notify.
- added test_19_* for shutdown related tests
- test_19_01 and test_19_02 test for TCP RST packets
which happen without a graceful shutdown and should
no longer appear otherwise.
- add test_19_03 for handling shutdowns by the server
- add test_19_04 for handling shutdowns by curl
- add test_19_05 for event based shutdowny by server
- add test_30_06/07 and test_31_06/07 for shutdown checks
on FTP up- and downloads.
Closes#13976
- in phase CONNECTING/TUNNELING/PROTOCONNECT, retrieve
the socket from the connection filters and do not rely
on `conn->sockfd` being already set by the transfer.
- this applies to the default behaviour, a protocol handler
may override this via its callbacks.
- add a warning message in multi_getsock() when the transfer
is expected to have something in its pollset, but instead
it is empty.
Reported-by: saurabhsingh-dev on github
Fixes#13998Closes#14011
- When a transfer sets `data->state.select_bits`, it is
scheduled for rerun with EXPIRE_NOW. If such a transfer
is blocked (due to PAUSE, for example), this will lead to
a busy loop.
- multi.c: check for transfer block
- sendf.*: add Curl_xfer_is_blocked()
- sendf.*: add client reader `is_paused()` callback
- implement is_paused()` callback where needed
Closes#13908
- new struct curl_pollfds and struct curl_waitfds
- add structs and methods to init/add/cleanup an array of pollfd and
struct curl_waitfd. Use in multi_wait() and multi_waitfds() to
populate the sets for polling.
- place USE_WINSOCK WSAEventSelect() setting into a separate loop over
all collected pfds
Closes#13900
