Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

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

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

Aulddays opened this issue Jan 7, 2017 · 7 comments

Comments

@Aulddays
Copy link
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
Copy link
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 added a commit to Aulddays/curl that referenced this issue Jan 7, 2017
@Aulddays
[area]: Improve the asiohiper example
5f78662 
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 curl#1191]
[Fixed by: Mingliang Zhu]
@Aulddays Aulddays mentioned this issue Jan 7, 2017
Closed
@Aulddays
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
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
Copy link
Contributor Author

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 closed this as completed in ed2fcd5 Jan 8, 2017
@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bagder @Aulddays