Re: Infinite loop when calling curl_multi_cleanup

[Dan Fandrich]

On Fri, Oct 21, 2016 at 11:13:04AM +0200, Daniel Stenberg wrote:
> I propose this change (see attachment) that "forces" the connections to get
> marked for closure before tryign to do disconnect them, which should fix the
> problems we've now got reported three times.
> 
> It isn't really a fix to avoid us ending up in this situation but more how
> to handle it when we do, which is lame but hopefully at least a step toward
> fixing hanging problems for people.
> 
> Unless someone objects, I will merge this within a few days.

This fixes my issue with test 1500.

>>> Dan
---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Re: Infinite loop when calling curl_multi_cleanup

[Valentin David]

Sorry for answering late. I did not tweak my email filter after
subscribing.

On Tue, 2016-10-18 at 10:26 +0200, Daniel Stenberg wrote:
> Infinite loop sounds related to Dan F's post from two weeks ago: 
> https://curl.haxx.se/mail/lib-2016-10/0011.html, but the triggering
> reason 
> seems different(?)
> 
> How did you end up in this situation, is it reproducible? Are you
> using 
> Pipelining or HTTP/2 multiplexing?


No HTTP/2. But maybe some pipelining. Howerver I do not think it is
used when this happens. recv_pipe size is 1. I think the way it happens
is there is a request sent, but no answer received yet. We remove the
easy from the multi. Then we close the multi. But there might not have
been any curl_multi_socket_action called between.

-- 
Valentin David


---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Re: Infinite loop when calling curl_multi_cleanup

[Ray Satiro via curl-library]

On 10/18/2016 7:16 PM, Miloš Ljumović wrote:
"Handles. You must never share the same handle in multiple threads. 
You can pass the handles around among threads, but you must never use 
a single handle from more than one thread at any given time."
I'm not sharing handles. Take a look at ServerProc. That's the thread 
responsible for multi_* interface. It's the only thread operating on 
hMultiHandle without protection. While other threads that uses 
PushRequest are locking the section where they use hMultiHandle. If 
libcurl is thread safe then my concept is fine.


"Also, I notice you didn't give your curl_version() information. 
libcurl is continuously improving, and so is nghttp2. I would try the 
latest of both."


7.51.0 - DEV
openssl - 1.0.2h
nghttp2 1.15.0

As I said - I have no problems. Program is working fine. After I added 
"connclose" in close_all_connections as suggested at the begging of 
this email chain - everything worked fine.


Though I was worried that maybe I haven't implemented:
multi_perform
multi_wait
multi_read

in a way they were intended. That's all.
Try the program (in the attachment) and we'll continue discussion. 


I don't have Apple APNs to try this program. I reiterate that it appears 
you are locking around adding and removing from the multi handle, but 
not the other calls that use that handle. Any action on a handle is use 
of a handle. "you must never use a single handle from more than one 
thread at any given time."


Concerning your use of curl_multi_wait:

"If no extra file descriptors are provided and libcurl has no file 
descriptor to offer to wait for, this function will return immediately." [1]


So if that happens it's not going to wait up to the 100ms you set. The 
'EXAMPLE' section of that document shows how it's typically handled: If 
numfds is 0 two or more times in a row then sleep 100 milliseconds.


Another thing is long options you are setting to 0 instead set to 0L. 
Although in Windows either will work since sizeof(int)==sizeof(long) 
it's not good practice in c variable argument functions (like 
curl_easy_setopt) to pass a type different from what the option/format 
specifies.



[1]: https://curl.haxx.se/libcurl/c/curl_multi_wait.html

---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Re: Infinite loop when calling curl_multi_cleanup

[Daniel Stenberg]

On Wed, 19 Oct 2016, Miloš Ljumović wrote:


Try the program (in the attachment) and we'll continue discussion.


One quick note (its getting late here): this isn't using pipelning, it enables 
HTTP/2 multiplexing.


--

 / daniel.haxx.se---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Re: Infinite loop when calling curl_multi_cleanup

[Miloš Ljumović]

"Your example is not complete, but based on what you have provided here 
is what you should look at:"

Yes - as I said, pseudo code. It was meant to give you an idea.

"Strings passed to libcurl as 'char *' arguments, are copied by the 
library" [1] with the exception of CURLOPT_POSTFIELDS."

This is what I needed to know. Thanks!

"If your program is crashing when you delete the buffer after setting it 
as the URL then the problem is elsewhere."

Program is not crashing. Try it.

"Handles. You must never share the same handle in multiple threads. You 
can pass the handles around among threads, but you must never use a 
single handle from more than one thread at any given time."
I'm not sharing handles. Take a look at ServerProc. That's the thread 
responsible for multi_* interface. It's the only thread operating on 
hMultiHandle without protection. While other threads that uses 
PushRequest are locking the section where they use hMultiHandle. If 
libcurl is thread safe then my concept is fine.


"Also, I notice you didn't give your curl_version() information. libcurl 
is continuously improving, and so is nghttp2. I would try the latest of 
both."


7.51.0 - DEV
openssl - 1.0.2h
nghttp2 1.15.0

As I said - I have no problems. Program is working fine. After I added 
"connclose" in close_all_connections as suggested at the begging of this 
email chain - everything worked fine.


Though I was worried that maybe I haven't implemented:
multi_perform
multi_wait
multi_read

in a way they were intended. That's all.
Try the program (in the attachment) and we'll continue discussion.

--
Miloš Ljumović
Operating systems specialist Spec.App.
http://milos.expert.its.me


#define HAVE_BOOL_T

#ifndef __CURL_INCLUDED__
#define __CURL_INCLUDED__
#include 
#include 
#include 
#endif

#define PROXY_URL   "YOUR PROXY URL:PORT GOES HERE"
#define PROXY_CREDENTIALS   "YOUR PROXY USER:PASS GOES HERE"

#define CERT_PATH   "YOUR CERTIFICATE PATH GOES HERE"
#define CERT_KEYPATH"YOUR CERTIFICATE KEY PATH GOES HERE"
#define CERT_CAPATH "YOUR CERTIFICATE CA PATH GOES HERE"
#define CERT_PASSWORD   "YOUR CERTIFICATE PASSWORD GOES HERE"

#define APNS_TOPIC  "YOUR APNS TOPIC GOES HERE"

#include 

class CTransfers
{
public:
CTransfers(void) : lTransfers(0)
{
hEvent = CreateEvent(0, TRUE, FALSE, 
"__12345_PUSHTRANSFER_67890__");
}

~CTransfers(void)
{
SetEvent(hEvent);
WaitForSingleObject(hEvent, 10);
CloseHandle(hEvent);
}

long operator++(int)
{
InterlockedIncrement();
return Get();
}

long operator--(int)
{
InterlockedDecrement();
return Get();
}

long operator=(long lValue)
{
InterlockedExchange(, lValue);
return Get();
}

long Get(void)
{
long lResult = InterlockedExchangeAdd(, 0);
if (lResult == 0)
{
ResetEvent(hEvent);
}
else if (lResult == 1)
{
SetEvent(hEvent);
}
return lResult;
}

void Pulse(void)
{
SetEvent(hEvent);
}

void WaitInput(void)
{
WaitForSingleObject(hEvent, INFINITE);
}
private:
HANDLE hEvent;
long lTransfers;
};

typedef long (__cdecl* RESPONSECALLBACK)(long lResponseCode, char* 
szDeviceToken);

class CPushResponse
{
public:
CPushResponse(long lResponseCode, char* szDeviceToken)
{
ResponseCode = lResponseCode;
char* pszDeviceToken = strrchr(szDeviceToken, '/');
if (pszDeviceToken)
{
strcpy(DeviceToken, ++pszDeviceToken);
}
else
{
strcpy(DeviceToken, szDeviceToken);
}
}
public:
long ResponseCode;
char DeviceToken[128];
};

class CPushService
{
public:
CPushService(RESPONSECALLBACK fnResponseCallback) : 
ResponseCallback(fnResponseCallback)
{
// 
SecureZeroMemory(szOutputBuffer, 4096);

// MD5 hash: [__SERVERREADY_20161015180622331__]
pszReadyEvent = "362c4945d1c91cb8563d7678a5f09930";
// MD5 hash: [__SERVEREXIT_20161016114452997__]
pszExitEvent = "fe9ff286fcb8f9c19522b12b6f05677a";
// MD5 hash: [__DISPATCHEREVENT_2016101704133__]
pszDispatcherEvent = "f8eb63cc78ae0fd950021662f5aca3cd";

// 
InitializeLocks();

// 

Re: Infinite loop when calling curl_multi_cleanup

[Ray Satiro via curl-library]

On 10/18/2016 12:08 PM, Miloš Ljumović wrote:
// Here I have to use dynamic memory - strange, cause I saw 
"strdup" in libcurl

char* szUrlBuffer = new char[1024]; // leak, needs upgrade
StringCchPrintf(szUrlBuffer, 1024, "%s%s", szAPNSAddress, 
szDeviceToken);

curl_easy_setopt(hEasyHandle, CURLOPT_URL, szUrlBuffer);
//- 
curl_easy_setopt(hEasyHandle, CURLOPT_HTTP_VERSION, 
CURL_HTTP_VERSION_2_0);

curl_easy_setopt(hEasyHandle, CURLOPT_SSLCERT, ...);
...
curl_easy_setopt(hEasyHandle, CURLOPT_USERAGENT, "curl/7.51.0-DEV");
curl_easy_setopt(hEasyHandle, CURLOPT_TCP_KEEPALIVE, 1L);
curl_easy_setopt(hEasyHandle, CURLOPT_PIPEWAIT, 1L);

Lock();
{
curl_multi_add_handle(hMultiHandle, hEasyHandle);
}
Unlock();


Your example is not complete, but based on what you have provided here 
is what you should look at:


"Strings passed to libcurl as 'char *' arguments, are copied by the 
library" [1] with the exception of CURLOPT_POSTFIELDS.


If your program is crashing when you delete the buffer after setting it 
as the URL then the problem is elsewhere.


"Handles. You must never share the same handle in multiple threads. You 
can pass the handles around among threads, but you must never use a 
single handle from more than one thread at any given time." [2]


It would appear that though you are locking with a critical section when 
you add and remove multi handles you are not locking around other calls 
where the multi is in use. Adding a critical section around every multi 
call in your perform thread is inefficient and will lead to more waiting 
than necessary in the threads that are adding. The way I would do it is 
create a thread-safe queue of prepared easy handles and then 
multi_add_handle in the thread that is using the multi handle.


Also, I notice you didn't give your curl_version() information. libcurl 
is continuously improving, and so is nghttp2. I would try the latest of 
both.


If you still have problems after accounting for these things please put 
together a minimal self-contained example that compiles and reproduces 
the problem.



[1]: https://curl.haxx.se/libcurl/c/curl_easy_setopt.html
[2]: https://curl.haxx.se/libcurl/c/threadsafe.html

---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Re: Infinite loop when calling curl_multi_cleanup

[Miloš Ljumović]

Here is the (pseudo) code:

long PushRequest(params ...)
{
CURL* hEasyHandle = 0;
Lock(); // EnterCriticalSection
{
// init easy handle.
hEasyHandle = curl_easy_init();
}
Unlock(); // LeaveCriticalSection

if (!hEasyHandle)
{
return -1;
}

curl_easy_setopt(...

//- 
// Here I have to use dynamic memory - strange, cause I saw 
"strdup" in libcurl

char* szUrlBuffer = new char[1024]; // leak, needs upgrade
StringCchPrintf(szUrlBuffer, 1024, "%s%s", szAPNSAddress, 
szDeviceToken);

curl_easy_setopt(hEasyHandle, CURLOPT_URL, szUrlBuffer);
//- 
curl_easy_setopt(hEasyHandle, CURLOPT_HTTP_VERSION, 
CURL_HTTP_VERSION_2_0);

curl_easy_setopt(hEasyHandle, CURLOPT_SSLCERT, ...);
...
curl_easy_setopt(hEasyHandle, CURLOPT_USERAGENT, "curl/7.51.0-DEV");
curl_easy_setopt(hEasyHandle, CURLOPT_TCP_KEEPALIVE, 1L);
curl_easy_setopt(hEasyHandle, CURLOPT_PIPEWAIT, 1L);

Lock();
{
curl_multi_add_handle(hMultiHandle, hEasyHandle);
}
Unlock();
return 0L;
}

DWORD WINAPI ServerProc(LPVOID hParameter)
{
...
long response = 0;
char* buffer = 0;
CURLMcode mcResult = CURLM_OK;
while (event in non-signaled state)
{
// here i have logic which waits if no transfers - i don't want 
to bother you with synchronization

int numfds = 0;
int iStillRunning = 0;
CURLMcode result = CURLM_OK;
if ((result = curl_multi_perform(hMultiHandle, )) 
== CURLM_OK)

{
curl_multi_wait(hMultiHandle, NULL, 0, 100, );
}
do
{
int msgq = 0;
hMsg = curl_multi_info_read(hMultiHandle, );
if (hMsg && (hMsg->msg == CURLMSG_DONE))
{
curl_easy_getinfo(hMsg->easy_handle, 
CURLINFO_RESPONSE_CODE, );
curl_easy_getinfo(hMsg->easy_handle, 
CURLINFO_EFFECTIVE_URL, );

// enqueue message to dispatcher
...
// clean-up
Lock();
{
curl_multi_remove_handle(hMultiHandle, 
hMsg->easy_handle);

}
Unlock();
curl_easy_cleanup(hMsg->easy_handle);
}
} while(hMsg);
}
return 0L;
}

Clients are calling the PushRequest in parallel, while separate thread 
is running the ServerProc.
It works fine, I was able to push 100+ concurrent requests towards the 
Apple APNs without any problems.

Notifications were delivered.

Complete log running 4 threads is in the attachment.
I'm suspecting I'm doing something wrong, cause I'm getting strange 
messages in log (sample):

* Server doesn't support multi-use yet, wait
* Curl_readwrite: forcibly told to drain data
...

Thanks!

--
Miloš Ljumović
Operating systems specialist Spec.App.
http://milos.expert.its.me

* STATE: INIT => CONNECT handle 0xf3ef20; line 1397 (connection #-5000)
* Found bundle for host api.push.apple.com: 0xf675c0 [serially]
* Server doesn't support multi-use yet, wait
* No connections available.
* STATE: CONNECT => CONNECT_PEND handle 0xf3ef20; line 1416 (connection #-5000)
* STATE: INIT => CONNECT handle 0xf52e50; line 1397 (connection #-5000)
* Found bundle for host api.push.apple.com: 0xf675c0 [serially]
* Server doesn't support multi-use yet, wait
* No connections available.
* STATE: CONNECT => CONNECT_PEND handle 0xf52e50; line 1416 (connection #-5000)
* STATE: INIT => CONNECT handle 0xf5ccb8; line 1397 (connection #-5000)
* Found bundle for host api.push.apple.com: 0xf675c0 [serially]
* Server doesn't support multi-use yet, wait
* No connections available.
* STATE: CONNECT => CONNECT_PEND handle 0xf5ccb8; line 1416 (connection #-5000)
* STATE: CONNECT_PEND => CONNECT handle 0xf3ef20; line 3080 (connection #-5000)
* STATE: CONNECT_PEND => CONNECT handle 0xf52e50; line 3080 (connection #-5000)
* STATE: CONNECT_PEND => CONNECT handle 0xf5ccb8; line 3080 (connection #-5000)
* Found bundle for host api.push.apple.com: 0xf675c0 [can multiplex]
* Conn: 0 (0xf66b20) Receive pipe weight: (-1/0), penalized: (nil)
* Multiplexed connection found!
* Found connection 0, with requests in the pipe (1)
* Re-using existing connection! (#0) with proxy **
* STATE: CONNECT => DO handle 0xf3ef20; line 1443 (connection #0)
* http2_send len=274
* h2 header: :method:POST
* h2 header: :path:/3/device/**
* h2 header: :scheme:https
* h2 header: :authority:api.push.apple.com
* h2 header: User-Agent:curl/7.51.0-DEV
* h2 header: Accept:*/*
* h2 header: **
* h2 header: Content-Length:46
* h2 header: Content-Type:application/x-www-form-urlencoded
* Using Stream ID: 3 (easy handle 0xf3ef20)
* before_frame_send() was called
* on_frame_send() was called, length = 70
> POST /3/device/** HTTP/1.1
Host: api.push.apple.com
User-Agent: curl/7.51.0-DEV
Accept: */*
apns-topic: 

Re: Infinite loop when calling curl_multi_cleanup

[Daniel Stenberg]

On Tue, 18 Oct 2016, Miloš Ljumović wrote:


Gets stuck. 7.51.0.


There is no 7.51.0 released yet, so then you must be using libcurl built 
straight from git.


But yeah, it makes sense that the bug is still in the code even after 7.50.3 
since there have been no attempts of fixing it yet.


Any chance you can debug that close connections loop to see what the exact 
reason is why it leaves the connections alive? I'd say it sounds likely that 
you also have the recv_pipe problem.



What do you mean "don't top post"?


If you instead had asked google that, you'd already know and you would've 
saved me having to paste this link, which btw is also included in the footer 
of every mail that is sent to this mailing list.


  https://curl.haxx.se/mail/etiquette.html#Do_Not_Top_Post

--

 / daniel.haxx.se---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Re: Infinite loop when calling curl_multi_cleanup

[Miloš Ljumović]

Gets stuck. 7.51.0. 

What do you mean "don't top post"?

Miloš Ljumović
Operating system specialist 
http://milos.expert.its.me

> On 18 Oct 2016, at 11:21, Daniel Stenberg  wrote:
> 
> On Tue, 18 Oct 2016, Miloš Ljumović wrote:
> 
> Please don't top-post.
> 
>> I have the same situation. I'm using pipelining. But it's different scenario.
> 
> You mean same as in it gets stuck or same as in still contains receivers in 
> recv_pipe ? Also on 7.50.3? (Not that I think we changed anything particular 
> in this area lately so this problem is likely to also exist in other 
> versions.)
> 
>> I need to polish up the code and then I'll share it cause though it works 
>> perfectly I'm not sure multi_* it's meant to work like that.
> 
> Certainly no function is ever meant to get stuck in an infinite loop? So no 
> it isn't intended.
> 
>> P.s. Where should I upload/share the code? Here?
> 
> Sure or in a pastebin, github gist or whatever you think is convenient. And 
> then I assume that the code is reasonably small and easy to understand/use 
> too, as otherwise there's not much of a point.
> 
> -- 
> 
> / daniel.haxx.se
> ---
> List admin: https://cool.haxx.se/list/listinfo/curl-library
> Etiquette:  https://curl.haxx.se/mail/etiquette.html


---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Re: Infinite loop when calling curl_multi_cleanup

[Daniel Stenberg]

On Tue, 18 Oct 2016, Miloš Ljumović wrote:

Please don't top-post.

I have the same situation. I'm using pipelining. But it's different 
scenario.


You mean same as in it gets stuck or same as in still contains receivers in 
recv_pipe ? Also on 7.50.3? (Not that I think we changed anything particular 
in this area lately so this problem is likely to also exist in other 
versions.)


I need to polish up the code and then I'll share it cause though it works 
perfectly I'm not sure multi_* it's meant to work like that.


Certainly no function is ever meant to get stuck in an infinite loop? So no it 
isn't intended.



P.s. Where should I upload/share the code? Here?


Sure or in a pastebin, github gist or whatever you think is convenient. And 
then I assume that the code is reasonably small and easy to understand/use 
too, as otherwise there's not much of a point.


--

 / daniel.haxx.se---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Re: Infinite loop when calling curl_multi_cleanup

[Miloš Ljumović]

I have the same situation. I'm using pipelining. But it's different scenario.
One thread does all multi_* calls while all other threads do easy_init & 
multi_add/remove_handle protected by the critical section. Everything works 
perfectly on 100 parallel threads but close_all_connections never returned. It 
looked like 2 connections remained in the list all the time. I need to polish 
up the code and then I'll share it cause though it works perfectly I'm not sure 
multi_* it's meant to work like that. 

P.s. Where should I upload/share the code? Here?

Miloš Ljumović
Operating system specialist 
http://milos.expert.its.me

> On 18 Oct 2016, at 10:26, Daniel Stenberg  wrote:
> 
>> On Mon, 17 Oct 2016, Valentin David wrote:
>> 
>> When I upgraded cURL to version 7.50.3, I got some of my code to run into an 
>> infinite loop when closing down the "multi". All "easy" are closed, but some 
>> connections in the cache still contains receivers in the "recv_pipe". 
>> Calling "Curl_disconnect" would not close properly the connection and 
>> "close_all_connections" would call it in loop forever. Adding "connclose" 
>> before call the "Curl_disconnect" fixes the problem.
> 
> Infinite loop sounds related to Dan F's post from two weeks ago: 
> https://curl.haxx.se/mail/lib-2016-10/0011.html, but the triggering reason 
> seems different(?)
> 
> How did you end up in this situation, is it reproducible? Are you using 
> Pipelining or HTTP/2 multiplexing?
> 
> While I think we should or could do that closing down sequence use more force 
> since it is about to close everything down anyway, it feels like there's a 
> bug lurking here that we should rather fix first than just trying to paint 
> over the problem by forcing the closure of that connection.
> 
> -- 
> 
> / daniel.haxx.se
> ---
> List admin: https://cool.haxx.se/list/listinfo/curl-library
> Etiquette:  https://curl.haxx.se/mail/etiquette.html


---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Re: Infinite loop when calling curl_multi_cleanup

[Daniel Stenberg]

On Mon, 17 Oct 2016, Valentin David wrote:

When I upgraded cURL to version 7.50.3, I got some of my code to run into an 
infinite loop when closing down the "multi". All "easy" are closed, but some 
connections in the cache still contains receivers in the "recv_pipe". 
Calling "Curl_disconnect" would not close properly the connection and 
"close_all_connections" would call it in loop forever. Adding "connclose" 
before call the "Curl_disconnect" fixes the problem.


Infinite loop sounds related to Dan F's post from two weeks ago: 
https://curl.haxx.se/mail/lib-2016-10/0011.html, but the triggering reason 
seems different(?)


How did you end up in this situation, is it reproducible? Are you using 
Pipelining or HTTP/2 multiplexing?


While I think we should or could do that closing down sequence use more force 
since it is about to close everything down anyway, it feels like there's a bug 
lurking here that we should rather fix first than just trying to paint over 
the problem by forcing the closure of that connection.


--

 / daniel.haxx.se
---
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html