Without a way to reproduce or figure out the internal state at the time of the crash this is really hard for us to handle. libcurl has a system to avoid calling the callback again for the same timeout. This rather looks like the time tree is damaged or something.

Have you made sure that you're not accessing any libcurl handles simultaneously from more than one thread?

Without a way to reproduce or figure out the internal state at the time of the crash this is really hard for us to handle.

Yes, I am aware of that. Unfortunately, this appears to happen randomly

libcurl has a system to avoid calling the callback again for the same timeout. This rather looks like the time tree is damaged or something.

I had the opportunity to look into curl code. From my understanding I can't see how that mechanism will stop the callback from being called multiple times due to this piece of code:

else {
    /* Asked to run due to time-out. Clear the 'lastcall' variable to force
       update_timer() to trigger a callback to the app again even if the same
       timeout is still the one to run after this call. That handles the case
       when the application asks libcurl to run the timeout prematurely. */
    memset(&multi->timer_lastcall, 0, sizeof(multi->timer_lastcall));
}

maybe it is handled somewhere else?

Have you made sure that you're not accessing any libcurl handles simultaneously from more than one thread?

Yes, libcurl is running in 1 thread only and on top of that it is stranded. This started to occur when we upgraded libcurl from version 7.58.0 to 7.63.0, not sure if it is related.

I had the opportunity to look into curl code. From my understanding I can't see how that mechanism will stop the callback from being called multiple times due to this piece of code:

Sure, but that logic is there to handle when libcurl is called to handle a timeout before the timeout has actually expired internally. Since before that time, the necessary actions won't be performed and thus libcurl wants to get told again when the timeout actually has expired. That shouldn't be possible to occur in a loop, unless I'm missing something.

Additionally: even when this happens in a rapid loop (which of course it shouldn't), that still shouldn't be a reason for a crash. You seem to call libcurl back again in the callback that curl tells you about the new timeout in. What if you don't do that and instead call it using the regular timeout handling so that you don't get this recursive effect? Or as a test, consider "now" (the zero value) to be 1 millisecond and see if that changes anything. libcurl counts expiry based on milliseconds internally, could that loop happen within the same millisecond? Do we get at least millisecond accuracy on the timer in iOS?

This started to occur when we upgraded libcurl from version 7.58.0 to 7.63.0

Lots of things have changed in curl in that time period, but we haven't changed any behavior in this area - at least not on purpose.

Additionally: even when this happens in a rapid loop (which of course it shouldn't), that still shouldn't be a reason for a crash. You seem to call libcurl back again in the callback that curl tells you about the new timeout in. What if you don't do that and instead call it using the regular timeout handling so that you don't get this recursive effect?

This is a copy of the CURLMOPT_TIMERFUNCTION callback:

//CURLMOPT_TIMERFUNCTION
static int curlMultiTimerCB(CURLM *multi, long timeoutMS, NetworkManager* nmanager)
{
	nmanager->m_asioTimer.cancel();

	if (timeoutMS > 0)
	{
		nmanager->m_asioTimer.expires_from_now(boost::posix_time::millisec(timeoutMS));
		nmanager->m_asioTimer.async_wait([nmanager](const boost::system::error_code& error)
		{
			if (error)
				return;

			int stillRunningHandles;
			auto ccode = curl_multi_socket_action(nmanager->m_curlMulti, CURL_SOCKET_TIMEOUT, 0, &stillRunningHandles);

			std::lock_guard<std::mutex> lock(nmanager->m_mutex);
			nmanager->checkCURLHandles();
		});

		return 0;
	}

	if (timeoutMS == 0) //success
	{
		int stillRunningHandles;
		auto ccode = curl_multi_socket_action(nmanager->m_curlMulti, CURL_SOCKET_TIMEOUT, 0, &stillRunningHandles);

		nmanager->checkCURLHandles();
	}

	return 0;
}

can you identify something wrong in the code? anything that can be improved? checkCURLHandles
checks for completed transfers to cleanup curl handles, that is, it calls in loop curl_multi_info_read in search for msg == CURLMSG_DONE

Or as a test, consider "now" (the zero value) to be 1 millisecond and see if that changes anything.

Note that curl documentation tells that with a timeout_ms value of 0 we should call curl_multi_socket_action and that's exactly what we are doing as shown in the above code. In such cases curl 7.63.0 will fallback to that else which memsets the multi->timer_lastcall. Also, in local tests seems that it is expected for curl to call the callback multiple times (~10 consecutive times) with timeout_ms = 0 but I guess this is curl detecting other successful requests.

libcurl counts expiry based on milliseconds internally, could that loop happen within the same millisecond? Do we get at least millisecond accuracy on the timer in iOS?

I think it is up to curl to ensure that millisecond accuracy is guaranteed in iOS. This is the thing that bothers me the most, this loop is not happening in Android nor Windows for some reason.

Reference: https://curl.haxx.se/libcurl/c/CURLMOPT_TIMERFUNCTION.html

(Was digging into a different thing but I noticed this issue and know the answer so I may as well answer)

Note that curl documentation tells that with a timeout_ms value of 0 we should call curl_multi_socket_action and that's exactly what we are doing as shown in the above code.

This may be a misinterpretation and/or miswording of the documentation. You're supposed to call the function from the next iteration of your event loop, instead of calling it immediately. The same thing would happen if you actually did set a 0 ms timer, and the documentation could be worded as "0 is not a special value: a timer should be set whenever timeout_ms is not -1".

This may be a misinterpretation and/or miswording of the documentation

Maybe, but calling back like that is done by example already and should probably work. I asked @rgoomes to try the other approach just to see if that changes anything or not.

@rgoomes wrote:

I think it is up to curl to ensure that millisecond accuracy is guaranteed in iOS.

I don't know iOS and I don't know what time accuracy it provides. I was hoping someone who works on that platform could just confirm it for us. libcurl is highly portable and runs on way more platforms than any of us in the core team owns or masters ourselves...

This may be a misinterpretation and/or miswording of the documentation

Maybe, but calling back like that is done by example already and should probably work. I asked @rgoomes to try the other approach just to see if that changes anything or not.

Yes, assuming a timeout of 0 as 1 millisecond apparently fixes the problem. However, I don't like this workaround of delaying the process by 1 millisecond.

Have you made sure that you're not accessing any libcurl handles simultaneously from more than one thread?

Yes, libcurl is running in 1 thread only and on top of that it is stranded. This started to occur when we upgraded libcurl from version 7.58.0 to 7.63.0, not sure if it is related.

Just to be clear here you're saying basically you have asynchronous callback but they are set to run sequentially in the same thread (ie stranded)? Like what thread is the async call to curl_multi_socket_action and who's to say that's not also running in another thread and where is the synchronization? We don't guarantee accessing the same handle from more than one thread at a time, libcurl doesn't have internal thread synchronization you must provide that on your own. Review thread safety. This is the only report I've heard of this and so I think it may be your code. If this is entirely reproducible you can try bisecting it.

Yes, assuming a timeout of 0 as 1 millisecond apparently fixes the problem. However, I don't like this workaround of delaying the process by 1 millisecond.

Next attempt I suggest would be to follow @TvdW's advice and use the 0 as a normal timeout as well, so that it'll just be scheduled to run immediately next, but not called back directly from within the callback. That will avoid the recursive risk and at the same time avoid a 1 millisecond delay.

Yes, assuming a timeout of 0 as 1 millisecond apparently fixes the problem. However, I don't like this workaround of delaying the process by 1 millisecond.

Next attempt I suggest would be to follow @TvdW's advice and use the 0 as a normal timeout as well, so that it'll just be scheduled to run immediately next, but not called back directly from within the callback. That will avoid the recursive risk and at the same time avoid a 1 millisecond delay.

That approach also seems to work. Apparently, any approach that avoids the recursive risk is enough to fix the loop. However, everyone seems to be doing exactly the opposite, at least implementations using boost asio with curl, that is, they call immediately curl_multi_socket_action when the timeout is 0. I'm still not sure if this is wrong or otherwise there's a bug in libcurl.

everyone seems to be doing exactly the opposite, at least implementations using boost asio with curl, that is, they call immediately curl_multi_socket_action when the timeout is 0. I'm still not sure if this is wrong or otherwise there's a bug in libcurl.

There are several possible explanations to why you report this and not others:

1. it could just be that nobody else has reported it, even if the bug happened for them
2. it might be that the iOS timer isn't reporting full millisecond resolution and thus we're more likely to get called for the same time over and over, which means recursively if called from within the callback
3. there might be a smaller stack on iOS than other platforms using the event-based API, thus making it more "vulnerable" for deep recursive calls
4. there might be a libcurl bug or two that makes it get stuck on the same millisecond, that we haven't detected

We would need more debugging to figure out the truth.

Personally I'm not a fan of calling libcurl back from within its own callbacks. I think I'll update the examples doing this to stop them from encouraging that.

I'm still not sure if this is wrong or otherwise there's a bug in libcurl.

To reiterate "you must never use a single handle from more than one thread at any given time." If your async call is in a different thread then that could cause this issue. AFAIK forcing consecutive execution of the calls (what I assume you mean by stranded) doesn't guarantee they're all in the same thread, that will depend on your code. If you were going to debug this further I'd record the thread id/. Like why do you need that guard in the async function but not otherwise

I will suggest that we close this issue because we have not managed to reproduce it and thus nobody is working on a fix, there's a working work-around provided that makes it not trigger anymore and the examples are now modified to not encourage the pattern that triggered this issue in the first place.

Yes, you can close it. No more issues were found with the suggested advice.

@bagder are you aware of the analysis in issue tarantool/tarantool#4179 ? According to the issue, this commit #2419 seems to be the culprit of this behavior. Can you please confirm? Full analysis is in tarantool/tarantool@70821c6

Yes, that seems to be entirely correct and a fine analysis. The reason is quite simply the "optmization" I did in #2419 that now makes it possible to call the timer function again with a zero millisecond timeout and for many systems calling libcurl back again recursively will then risk this kind of stack smash.

I can't figure out a sensible or easy fix for this in another way than the way we already discussed and have documented.