This reverts commit cb3e6dfa35 and instead fixes the problem
differently.
The reverted commit addressed a test failure in test 1021 by simplifying
and generalizing the code flow in a way that damaged the
performance. Now we modify the flow so that Curl_proxyCONNECT() again
does as much as possible in one go, yet still do test 1021 with and
without valgrind. It failed due to mistakes in the multi state machine.
Bug: http://curl.haxx.se/bug/view.cgi?id=1397
Reported-by: Paul Saab
When an error has been detected, skip the final forced call to the
progress callback by making sure to pass the current return code
variable in the Curl_done() call in the CURLM_STATE_DONE state.
This avoids the "extra" callback that could occur even if you returned
error from the progress callback.
Bug: http://curl.haxx.se/mail/lib-2014-06/0062.html
Reported by: Jonathan Cardoso Machado
Make all code use connclose() and connkeep() when changing the "close
state" for a connection. These two macros take a string argument with an
explanation, and debug builds of curl will include that in the debug
output. Helps tracking connection re-use/close issues.
In commit 0b3750b5c2 (released in 7.36.0) we fixed a timeout issue
but instead broke the timings.
To fix this, I introduce a new timestamp to use for the timeouts and
restored the previous timestamp and timestamp position so that the old
timer functionality is restored.
In addition to that, that change also broke connection timeouts for when
more than one connect was used (as it would then count the total time
from the first connect and not for the most recent one). Now
Curl_timeleft() has been modified so that it checks against different
start times depending on which timeout it checks.
Test 1303 is updated accordingly.
Bug: http://curl.haxx.se/mail/lib-2014-05/0147.html
Reported-by: Ryan Braud
set.infilesize in this case was modified in several places, which could
lead to repeated requests using the same handle to get unintendent/wrong
consequences based on what the previous request did!
ufds might not be allocated in case nfds overflows to zero while
extra_nfds is still non-zero. udfs is then accessed within the
extra_nfds-based for loop.
Setting the TIMER_STARTSINGLE timestamp first in CONNECT has the
drawback that for actions that go back to the CONNECT state, the time
stamp is reset and for the multi_socket API there's no corresponding
Curl_expire() then so the timeout logic gets wrong!
Reported-by: Brad Spencer
Bug: http://curl.haxx.se/mail/lib-2014-02/0036.html
A transfer timeout could result in an error message such as "Operation
timed out after 3000 milliseconds with 19 bytes of -1 received". This
patch removes the non-sensical "of -1" when the size of the transfer
is unknown, mirroring the logic in lib/transfer.c
With the recently added timeout "reminder" functionality, there's no
reason left for us to execute timeout code before the time is
ripe. Simplifies the handling too.
This will make the *TIMEOUT and *CONNECTTIMEOUT options more accurate
again, which probably is most important when the *_MS versions are used.
In multi_socket, make sure to update 'now' after having handled activity
on a socket.
BACKGROUND:
We have learned that on some systems timeout timers are inaccurate and
might occasionally fire off too early. To make the multi_socket API work
with this, we made libcurl execute timeout actions a bit early too if
they are within our MULTI_TIMEOUT_INACCURACY. (added in commit
2c72732ebf, present since 7.21.0)
Switching everything to the multi API made this inaccuracy problem
slightly more notable as now everyone can be affected.
Recently (commit 21091549c0) we tweaked that inaccuracy value to make
timeouts more accurate and made it platform specific. We also figured
out that we have code at places that check for fixed timeout values so
they MUST NOT run too early as then they will not trigger at all (see
commit be28223f35 and a691e04470) - so there are definitately problems
with running timeouts before they're supposed to run. (We've handled
that so far by adding the inaccuracy margin to those specific timeouts.)
The libcurl multi_socket API tells the application with a callback that
a timeout expires in N milliseconds (and it explicitly will not tell it
again for the same timeout), and the application is then supposed to
call libcurl when that timeout expires. When libcurl subsequently gets
called with curl_multi_socket_action(...CURL_SOCKET_TIMEOUT...), it
knows that the application thinks the timeout expired - and alas, if it
is within the inaccuracy level libcurl will run code handling that
handle.
If the application says CURL_SOCKET_TIMEOUT to libcurl and _isn't_
within the inaccuracy level, libcurl will not consider the timeout
expired and it will not tell the application again since the timeout
value is still the same.
NOW:
This change introduces a modified behavior here. If the application says
CURL_SOCKET_TIMEOUT and libcurl finds no timeout code to run, it will
inform the application about the timeout value - *again* even if it is
the same timeout that it already told about before (although libcurl
will of course tell it the updated time so that it'll still get the
correct remaining time). This way, we will not risk that the application
believes it has done its job and libcurl thinks the time hasn't come yet
to run any code and both just sit waiting. This also allows us to
decrease the MULTI_TIMEOUT_INACCURACY margin, but that will be handled
in a separate commit.
A repeated timeout update to the application risk that the timeout will
then fire again immediately and we have what basically is a busy-loop
until the time is fine even for libcurl. If that becomes a problem, we
need to address it.
When the progress callback returned 1 at a very early state, the code
would not make CURLE_ABORTED_BY_CALLBACK get returned but the process
would still be interrupted. In the HTTP case, this would then cause a
CURLE_GOT_NOTHING to erroneously get returned instead.
Reported-by: Petr Novak
Bug: http://curl.haxx.se/bug/view.cgi?id=1318
Following commit 0aafd77fa4, replaced the internal usage of
FORMAT_OFF_T and FORMAT_OFF_TU with the external versions that we
expect API programmers to use.
This negates the need for separate definitions which were subtly
different under different platforms/compilers.
The FILE:// code doesn't support this option - and it doesn't make sense
to support it as long as it works as it does since then it'd only block
even longer.
But: setting CURLOPT_MAX_RECV_SPEED_LARGE would make the transfer first
get done and then libcurl would wait until the average speed would get
low enough. This happened because the transfer happens completely in the
DO state for FILE:// but then it would still unconditionally continue in
to the PERFORM state where the speed check is made.
Starting now, the code will skip from DO_DONE to DONE immediately if no
socket is set to be recv()ed or send()ed to.
Bug: http://curl.haxx.se/bug/view.cgi?id=1312
Reported-by: Mohammad AlSaleh
This is an extension to the fix in 7d80ed64e4. We may
call Curl_disconnect() while cleaning up the multi handle,
which could lead to openssl sending packets, which could get
a SIGPIPE.
Signed-off-by: Jeff King <peff@peff.net>
This patch fixes and issue introduced in commit 7d7df83198, if the
tunnel state was TUNNEL_CONNECT, waitconnect_getsock() would return a
bitmask indicating a readable socket but never stored the socket in the
return array.
This patch adds a 200ms delay between the first and second address
family socket connection attempts.
It also iterates over IP addresses in the order returned by the
system, meaning most dual-stack systems will try IPv6 first.
Additionally, it refactors the connect code, removing most code that
handled synchronous connects. Since all sockets are now non-blocking,
the logic can be made simpler.
The code rejected 0 as a valid timeout while in fact the function could
indeed legitimately return that and it should be respected.
Reported-by: Bjorn Stenberg
This patch invokes two socket connect()s nearly simultaneously, and
the socket that is first connected "wins" and is subsequently used for
the connection. The other is terminated.
There is a very slight IPv4 preference, in that if both sockets connect
simultaneously IPv4 is checked first and thus will win.
When waiting for a 100-continue response from the server, the
Curl_readwrite() will refuse to run if called until the timeout has been
reached.
We timeout code in multi_socket() allows code to run slightly before the
actual timeout time, so for test 154 it could lead to the function being
executed but refused in Curl_readwrite() and then the application would
just sit idling forever.
This was detected with runtests.pl -e on test 154.
Make sure we always return CURLM_CALL_MULTI_PERFORM when we reach
CURLM_STATE_DONE since the state is transient and it can very well
continue executing as there is nothing to wait for.
Bug: http://curl.haxx.se/mail/lib-2013-08/0211.html
Reported-by: Yi Huang
Doing curl_multi_add_handle() on an easy handle that is already added to
a multi handle now returns this error code. It previously returned
CURLM_BAD_EASY_HANDLE for this condition.
The closure_handle is "owned" by the multi handle and it is
unconditional so the setting up of it should be in the Curl_multi_handle
function rather than curl_multi_add_handle.
With everything being struct SessionHandle pointers now, this rename
makes multi.c use the library-wide practise of calling that pointer
'data' instead of the previously used 'easy'.
Moved Curl_easy_addmulti() from easy.c to multi.c, renamed it to
easy_addmulti and made it static.
Removed Curl_easy_initHandleData() and uses of it since it was emptied
in commit cdda92ab67b47d74a.
CURLOPT_DNS_USE_GLOBAL_CACHE broke in commit c43127414d (been
broken since the libcurl 7.29.0 release). While this option has been
documented as deprecated for almost a decade and nobody even reported
this bug, it should remain functional.
Added test case 1512 to verify
This is a regression as this logic used to work. It isn't clear when it
broke, but I'm assuming in 7.28.0 when we went all-multi internally.
This likely never worked with the multi interface. As the failed
connection is detected once the multi state has reached DO_MORE, the
Curl_do_more() function was now expanded somewhat so that the
ftp_do_more() function can request to go "back" to the previous state
when it makes another attempt - using PASV.
Added test case 1233 to verify this fix. It has the little issue that it
assumes no service is listening/accepting connections on port 1...
Reported-by: byte_bucket in the #curl IRC channel
The motivation for having a separate struct that keep track of an easy
handle when using the multi handle was removed when we switched to
always using the multi interface internally. Now they were just two
separate struct that was always allocated for each easy handle.
This first step just moves the Curl_one_easy struct members into the
SessionHandle struct and hides this somehow (== keeps the source code
changes to a minimum) by defining Curl_one_easy to SessionHandle
The biggest changes in this commit are:
1 - the linked list of easy handles had to be changed somewhat due
to the new struct layout. This made the main linked list pointer
get renamed to 'easyp' and there's also a new pointer to the last
node, called easylp. It is no longer circular but ends with ->next
pointing to NULL. New nodes are still added last.
2 - easy->state is now called easy->mstate to avoid name collision
Commit 6d30f8ebed didn't work properly. First, it used the wrong
array index, but this fix also:
1 - only does the copying if indeed there was any activity
2 - makes sure to properly translate between internal and external
bitfields, which are not guaranteed to match
Reported-by: Evgeny Turnaev
As a remedy to the problem when a socket gets closed and a new one is
opened with the same file descriptor number and as a result
multi.c:singlesocket() doesn't detect the difference, the new function
Curl_multi_closed() gets told when a socket is closed so that it can be
removed from the socket hash. When the old one has been removed, a new
socket should be detected fine by the singlesocket() on next invoke.
Bug: http://curl.haxx.se/bug/view.cgi?id=1248
Reported-by: Erik Johansson
Allow less room for "triggered too early" mistakes by applications /
timers on non-windows platforms. Starting now, we assume that a timeout
call is never made earlier than 3 milliseconds before the actual
timeout. This greatly improves timeout accuracy on Linux.
Bug: http://curl.haxx.se/bug/view.cgi?id=1228
Reported-by: Hang Su
commit 29bf0598aa introduced a problem when the "internal" timeout is
prefered to the given if shorter, as it didn't consider the case where
-1 was returned. Now the internal timeout is only considered if not -1.
Reported-by: Tor Arntsen
Bug: http://curl.haxx.se/mail/lib-2013-06/0015.html
