The asiohiper.cpp example code is not doing things quite right #1191

Closed
Aulddays opened this Issue Jan 7, 2017 · 7 comments

Projects

None yet

2 participants

@Aulddays
Contributor
Aulddays commented Jan 7, 2017

[ There are collections of known issues to be aware of:
https://curl.haxx.se/docs/knownbugs.html https://curl.haxx.se/docs/todo.html ]

There is an example code demostrating curl multi working with boost::asio at docs/examples/asiohiper.cpp and also at https://curl.haxx.se/libcurl/c/asiohiper.html

When you try to run the code, the download progress is noticably slower than expected (compared with a program that uses just an easy interface), especially if you change the download url from google homepage into some larger pages. To make that more noticable, modify this line in the code:
curl_easy_setopt(conn->easy, CURLOPT_LOW_SPEED_TIME, 3L);
Change "3L" to something larger, say 10L. Make that even larger may also cause a timeout or failure.

Even worse, if that line of code get commented out, the program will exit while only the beginning of page downloaded, and the "DONE: some_url => (0)" log line never got printed.

CURLOPT_LOW_SPEED_TIME just controls low speed detection, and should never cause download progress to be slow or fail (provided the network connection is not really that slow).

The problem is that the example uses CURLMOPT_SOCKETFUNCTION to connect libcurl to boost::asio. libcurl requires CURLMOPT_SOCKETFUNCTION to KEEP watching socket events and notify back. The example uses async_read_some() and async_write_some() to watch socket events. However, those two functions are both one-time watcher, so once some event has been fired, those two funcitons should be called again to meet the "keep watching" requirement of libcurl.

With the CURLOPT_LOW_SPEED_TIME removed, after first socket read event has fired, there will be no further things for boost::asio to do, so the program exits immediatly, leaving the url only partly downloaded. With CURLOPT_LOW_SPEED_TIME set, there will be a timer left in boost::asio for that, and that's why the the original code keeps slowly running but not exiting.

I'll provide a PR trying to fix this later.

@bagder
Member
bagder commented Jan 7, 2017

A request for help with exactly this example went out not too long ago so the fact it is not working correctly is known. We just haven't had anyone around with asio knowledge or experience to do it! We'll greatly appreciate your help!

@Aulddays Aulddays added a commit to Aulddays/curl that referenced this issue Jan 7, 2017
@Aulddays Aulddays [area]: Improve the asiohiper example
Fix the problem in asiohiper.cpp.
libcurl requires CURLMOPT_SOCKETFUNCTION to KEEP 
watching socket events and notify back. Modify 
event_cb() to continue watching events when fired.

[Bug: Fixes Issue #1191]
[Fixed by: Mingliang Zhu]
5f78662
@Aulddays
Contributor
Aulddays commented Jan 7, 2017

A test result for the commit. The exec time seems reasonable now and the downloaded content is correct.

$ time ./asiohiper
..........
DONE: HTTP://www.google.com/ => (0)
multi_timer_cb: timeout_ms -1
timer_cb:
REMAINING: 0
close_socket : 6
done.

real 0m3.941s
user 0m0.003s
sys 0m0.002s

$time ./asiohiper_mod
..........
DONE: http://www.google.com/ => (0)
multi_timer_cb: timeout_ms -1
timer_cb:
REMAINING: 0
last transfer done, kill timeout
close_socket : 6
done.

real 0m0.667s
user 0m0.001s
sys 0m0.007s

@Aulddays
Contributor
Aulddays commented Jan 7, 2017

@bagder I didn't notice those reports. I just read Issue 1170 and it looks like to be caused by the same problem. But the http request is a little more complicated. I'll take a close look if my commit fixes that.

@Aulddays
Contributor
Aulddays commented Jan 7, 2017

I further examined Issue #1170 and unfortunately it seems to be another problem than I have reported.
I ran the code provide by @sancane, the log shows it runs like this: libcurl asks asio to wait for the socket to get connected, then libcurl handles the SSL handshakes (libcurl handles these itself without asking asio, maybe this is the part mentioned in https://curl.haxx.se/libcurl/c/libcurl-multi.html as the "BLOCKING" part), then libssl explicitly asks asio to STOP watching the socket. As soon as asio stops watching anything, the program exists, which is expected in asio programs.
However, the behavior I examined is not quite the same as @sancane posted. He ran into a timeout, which I do not. This may be because incorrect SSL support in my environment.
@sancane did mention that the problem solved when he switched ghiper.c, I'll ask him for a working ghiper version and try to find the difference.

@Aulddays
Contributor
Aulddays commented Jan 7, 2017

Update: according to the logs by @sancane, the behavior I met looks like the one before @bagder mentioned a7b38c9. I'll try a7b38c9 later.

@bagder
Member
bagder commented Jan 7, 2017

libcurl handles these itself without asking asio, maybe this is the part mentioned in https://curl.haxx.se/libcurl/c/libcurl-multi.html as the "BLOCKING" part

No it isn't blocking at all (for most TLS backends at least) asio doesn't know about SSL so of course it has no particular knowledge or saying about the SSL handshake, it just gets socket activities and tells libcurl about them for SSL handshakes as well as any other socket stuff.

@Aulddays
Contributor
Aulddays commented Jan 8, 2017

It turns out that my problem get solved by switching libcurl to master.
And it seems that the code in Issue #1170 runs correctly now with my PR merged.

@bagder bagder added a commit that closed this issue Jan 8, 2017
@Aulddays @bagder Aulddays + bagder asiohiper: improved socket handling
libcurl requires CURLMOPT_SOCKETFUNCTION to KEEP watching socket events
and notify back. Modify event_cb() to continue watching events when
fired.

Fixes #1191
Closes #1192
Fixed-by: Mingliang Zhu
ed2fcd5
@bagder bagder closed this in ed2fcd5 Jan 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment