headers
Maxime Larocque | 10 Jul 2012 22:22
Picon
Favicon

patch - select.c / Curl_socket_check() interrupted

Hello,

We found a problem with ftp transfer using libcurl (7.23 and 7.25) 
inside an application which is receiving unix signals (SIGUSR1, 
SIGUSR2...) almost continuously. (Linux 2.4, PowerPC, HAVE_POLL_FINE 
defined).

Curl_socket_check() uses poll() to wait for the socket, and retries it 
when a signal is received (EINTR). However, if a signal is received and 
it also happens that the timeout has been reached, Curl_socket_check() 
returns -1 instead of 0 (indicating an error instead of a timeout).

In our case, the result is an aborted connection even before the ftp 
banner is received from the server, and a return value of 
CURLE_OUT_OF_MEMORY from curl_easy_perform() (Curl_pp_multi_statemach(), 
in pingpong.c, actually returns OOM if Curl_socket_check() fails :-) 
Funny to debug on a system on which OOM is a possible cause).

The following patch corrects the problem (for curl 7.26 release, but 
look the same as 7.25):

=======================================
diff -ur curl-7.26.0/lib/select.c curl-7.26.0b/lib/select.c
--- curl-7.26.0/lib/select.c    2012-03-08 14:35:25.000000000 -0500
+++ curl-7.26.0b/lib/select.c    2012-07-10 16:02:13.915651510 -0400
 <at>  <at>  -221,8 +221,10  <at>  <at> 
        break;
      if(timeout_ms > 0) {
        pending_ms = (int)(timeout_ms - elapsed_ms);
-      if(pending_ms <= 0)
(Continue reading)

Permalink | Reply |
headers
Daniel Stenberg | 7 Aug 2012 23:29
Picon
Favicon
Gravatar

Re: patch - select.c / Curl_socket_check() interrupted

On Tue, 10 Jul 2012, Maxime Larocque wrote:

> The following patch corrects the problem (for curl 7.26 release, but look 
> the same as 7.25):

Thanks a lot for the patch and your work on this, I finally got around to 
merge and push it!

> - Curl_pp_multi_statemach(), in pingpong.c which returns 
> CURLE_OUT_OF_MEMORY. Curl_socket_check() should not fail anyways, and I'm 
> not sure which code should be returned...

As 0 from Curl_socket_check() implies a timeout, isn't 
CURLE_OPERATION_TIMEDOUT the logical return code then?

> - the ! HAVE_POLL_FINE case in Curl_socket_check(); it uses select() but 
> seems to have the same trouble at select.c:309

Yes. I'll add the same fix there right now.

--

-- 

  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html
Permalink | Reply |
headers
Maxime Larocque | 13 Aug 2012 23:23
Picon
Favicon

Re: patch - select.c / Curl_socket_check() interrupted

Hi Daniel & the list,

Le 07/08/2012 17:29, Daniel Stenberg a écrit :
>
> Thanks a lot for the patch and your work on this, I finally got around 
> to merge and push it!

Thank you!

>> - Curl_pp_multi_statemach(), in pingpong.c which returns 
>> CURLE_OUT_OF_MEMORY. Curl_socket_check() should not fail anyways, and 
>> I'm not sure which code should be returned...
>
> As 0 from Curl_socket_check() implies a timeout, isn't 
> CURLE_OPERATION_TIMEDOUT the logical return code then?

When Curl_socket_check() returns 0 (ie timeout) or a positive value (ie 
a socket event), Curl_pp_multi_statemach() handles it correctly. When -1 
is returned by Curl_socket_check(), a CURLE_OUT_OF_MEMORY is returned to 
the user.

Curl_socket_check() will only return -1 when there is a bug causing 
select/poll to fail (which I got, the CURLE_OUT_OF_MEMORY led me to an 
incorrect path), or if select/poll really returned ENOMEM.

So, in short, the code in Curl_pp_multi_statemach() is correct, and it 
already has a failf() to report the select/poll error when in 
development mode...

Regards,
(Continue reading)

Permalink | Reply |

Gmane