Merge lp:~thomas-voss/net-cpp/fix-1423765 into lp:net-cpp

Proposed by Thomas Voß
Status: Merged
Approved by: Michi Henning
Approved revision: 54
Merged at revision: 46
Proposed branch: lp:~thomas-voss/net-cpp/fix-1423765
Merge into: lp:net-cpp
Diff against target: 484 lines (+149/-91)
4 files modified
debian/control (+0/-1)
src/core/net/http/impl/curl/easy.cpp (+19/-9)
src/core/net/http/impl/curl/easy.h (+38/-12)
src/core/net/http/impl/curl/multi.cpp (+92/-69)
To merge this branch: bzr merge lp:~thomas-voss/net-cpp/fix-1423765
Reviewer Review Type Date Requested Status
Michi Henning (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+250588@code.launchpad.net

Commit message

Make sure that Multi::Private instances are correctly cleaned up by only handing out weak_ptr's to it.

Description of the change

Make sure that Multi::Private instances are correctly cleaned up by only handing out weak_ptr's to it.

To post a comment you must log in.
Revision history for this message
Michi Henning (michihenning) wrote :

I just pulled the branch, ran cmake and successfully compiled it. Then, when running the tests, I got the famous "A requested feature, protocol or option was not found built-in in this libcurl due to a build-time decision." again.

net-cpp should not build if the modules it depends on are not installed. Could you please add whatever run-time dependencies there are to the build dependencies. I know we've been down this road once before, but I've forgotten what the missing packages are. And the error message that comes out of curl is singularly unhelpful. It would be sooo nice if the error message actually state *what* the thing is that couldn't be found :-(

review: Needs Fixing
Revision history for this message
Michi Henning (michihenning) wrote :

I'm still seeing some leaks. valgrind reports about 18 kB leaked for the http_client_test, and well over a megabyte for the load test.

review: Needs Fixing
lp:~thomas-voss/net-cpp/fix-1423765 updated
47. By Thomas Voß

Disable sharing to avoid memory leaks.

Revision history for this message
Thomas Voß (thomas-voss) wrote :
Download full text (3.8 KiB)

valgrind --leak-check=full for http_client_test now reports:

==30442==
==30442== HEAP SUMMARY:
==30442== in use at exit: 304 bytes in 10 blocks
==30442== total heap usage: 54,976 allocs, 54,966 frees, 4,334,461 bytes allocated
==30442==
==30442== LEAK SUMMARY:
==30442== definitely lost: 0 bytes in 0 blocks
==30442== indirectly lost: 0 bytes in 0 blocks
==30442== possibly lost: 0 bytes in 0 blocks
==30442== still reachable: 304 bytes in 10 blocks
==30442== suppressed: 0 bytes in 0 blocks
==30442== Reachable blocks (those to which a pointer was found) are not shown.
==30442== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==30442==
==30442== For counts of detected and suppressed errors, rerun with: -v
==30442== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

valgrind --leak-check=full for http_client_load_test now reports:

==30468==
==30468== HEAP SUMMARY:
==30468== in use at exit: 139,615 bytes in 1,125 blocks
==30468== total heap usage: 315,910 allocs, 314,785 frees, 54,715,917 bytes allocated
==30468==
==30468== 139,311 (152 direct, 139,159 indirect) bytes in 1 blocks are definitely lost in loss record 97 of 97
==30468== at 0x4C2B100: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30468== by 0x4E8FDDD: boost::asio::detail::epoll_reactor::descriptor_state* boost::asio::detail::object_pool_access::create<boost::asio::detail::epoll_reactor::descriptor_state>() (object_pool.hpp:35)
==30468== by 0x4E8D6AA: boost::asio::detail::object_pool<boost::asio::detail::epoll_reactor::descriptor_state>::alloc() (object_pool.hpp:89)
==30468== by 0x4E8985A: boost::asio::detail::epoll_reactor::allocate_descriptor_state() (epoll_reactor.ipp:512)
==30468== by 0x4E88D99: boost::asio::detail::epoll_reactor::register_descriptor(int, boost::asio::detail::epoll_reactor::descriptor_state*&) (epoll_reactor.ipp:151)
==30468== by 0x4E8BE6C: boost::asio::detail::reactive_descriptor_service::assign(boost::asio::detail::reactive_descriptor_service::implementation_type&, int const&, boost::system::error_code&) (reactive_descriptor_service.ipp:108)
==30468== by 0x4E8C1B6: boost::asio::posix::stream_descriptor_service::assign(boost::asio::detail::reactive_descriptor_service::implementation_type&, int const&, boost::system::error_code&) (stream_descriptor_service.hpp:116)
==30468== by 0x4E922D2: boost::asio::posix::basic_descriptor<boost::asio::posix::stream_descriptor_service>::basic_descriptor(boost::asio::io_service&, int const&) (basic_descriptor.hpp:90)
==30468== by 0x4E8F84E: boost::asio::posix::basic_stream_descriptor<boost::asio::posix::stream_descriptor_service>::basic_stream_descriptor(boost::asio::io_service&, int const&) (basic_stream_descriptor.hpp:91)
==30468== by 0x4E83FC9: curl::multi::Handle::Private::Socket::Private::Private(boost::asio::io_service&, int) (multi.cpp:463)
==30468== by 0x4E83E4E: curl::multi::Handle::Private::Socket::Socket(boost::asio::io_service&, int) (multi.cpp:441)
==30468== by 0x4E84741: curl::multi::Handle::Private::socket_callback(void*, int, int, void*, void*) (multi.cpp:577)
==30468==
=...

Read more...

lp:~thomas-voss/net-cpp/fix-1423765 updated
48. By Thomas Voß

Adjust build- and runtime deps for curl.

Revision history for this message
Michi Henning (michihenning) wrote :
Download full text (5.2 KiB)

Looks like most of the leaks are gone, thanks! But I'm still seeing problems. About one in every 10-15 runs of the load test on my 8-core Vivid machine, I get the output below from valgrind. Looks like there is a race condition somewhere.

==13111==
==13111== HEAP SUMMARY:
==13111== in use at exit: 174,592 bytes in 1,968 blocks
==13111== total heap usage: 317,581 allocs, 315,613 frees, 54,709,429 bytes allocated
==13111==
==13111== 174,528 (81 direct, 174,447 indirect) bytes in 1 blocks are definitely lost in loss record 65 of 65
==13111== at 0x4C2B100: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13111== by 0x4E878E7: boost::asio::detail::thread_info_base::allocate(boost::asio::detail::thread_info_base*, unsigned long) (thread_info_base.hpp:60)
==13111== by 0x4E879F6: boost::asio::asio_handler_allocate(unsigned long, ...) (handler_alloc_hook.ipp:50)
==13111== by 0x4E855F0: void* boost_asio_handler_alloc_helpers::allocate<curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}>(unsigned long, curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}&) (handler_alloc_helpers.hpp:37)
==13111== by 0x4E8513C: void boost::asio::detail::deadline_timer_service<boost::asio::time_traits<boost::posix_time::ptime> >::async_wait<curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}>(boost::asio::detail::deadline_timer_service<boost::asio::time_traits<boost::posix_time::ptime> >::implementation_type&, curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}&) (deadline_timer_service.hpp:185)
==13111== by 0x4E84E71: boost::asio::async_result<boost::asio::handler_type<curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}, void (boost::system::error_code)>::type>::type boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> >::async_wait<curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}>(boost::asio::detail::deadline_timer_service<boost::asio::time_traits<boost::posix_time::ptime> >::implementation_type&, boost::asio::handler_type&&) (deadline_timer_service.hpp:149)
==13111== by 0x4E84D63: boost::asio::async_result<boost::asio::handler_type<curl::multi::...

Read more...

review: Needs Fixing
Revision history for this message
Michi Henning (michihenning) wrote :

Regarding the issue with the dependencies, things work fine for me on my Vivid machine. On my Utopic machine, I can build the code but then still get the error when running the tests about a missing feature. I've checked that I have all the build deps from debian/control installed.

Either there is still something missing, or there is an issue with running the tests on Utopic?

lp:~thomas-voss/net-cpp/fix-1423765 updated
49. By Thomas Voß

Make sure that we do not keep instances of Private::Socket or Private::Timeout alive.
Clean up error reporting, and allow for transporting detailed error reports generated by curl via an exception.

50. By Thomas Voß

Allow for querying the current error description.

51. By Thomas Voß

Remove dep on libcurl3-gnutls to make tests work out of the box.

52. By Thomas Voß

Ensure that async writes are not keeping objects alive.

53. By Thomas Voß

Make curl's openssl dialect the default.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michi Henning (michihenning) wrote :
Download full text (5.0 KiB)

Running the load test in a loop, after maybe a hundred iterations or so, I got a valgrind complaint:

==29504==
==29504== HEAP SUMMARY:
==29504== in use at exit: 161,360 bytes in 1,769 blocks
==29504== total heap usage: 325,601 allocs, 323,832 frees, 54,917,164 bytes allocated
==29504==
==29504== 161,296 (73 direct, 161,223 indirect) bytes in 1 blocks are definitely lost in loss record 64 of 64
==29504== at 0x4C2B100: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==29504== by 0x4E8A699: boost::asio::detail::thread_info_base::allocate(boost::asio::detail::thread_info_base*, unsigned long) (thread_info_base.hpp:60)
==29504== by 0x4E8A7A8: boost::asio::asio_handler_allocate(unsigned long, ...) (handler_alloc_hook.ipp:50)
==29504== by 0x4E883DE: void* boost_asio_handler_alloc_helpers::allocate<curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}>(unsigned long, curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}&) (handler_alloc_helpers.hpp:37)
==29504== by 0x4E87F56: void boost::asio::detail::deadline_timer_service<boost::asio::time_traits<boost::posix_time::ptime> >::async_wait<curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}>(boost::asio::detail::deadline_timer_service<boost::asio::time_traits<boost::posix_time::ptime> >::implementation_type&, curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}&) (deadline_timer_service.hpp:185)
==29504== by 0x4E87CA1: boost::asio::async_result<boost::asio::handler_type<curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}, void (boost::system::error_code)>::type>::type boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> >::async_wait<curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::ratio<1l, 1000l> > const&)::{lambda(boost::system::error_code const&)#1}>(boost::asio::detail::deadline_timer_service<boost::asio::time_traits<boost::posix_time::ptime> >::implementation_type&, boost::asio::handler_type&&) (deadline_timer_service.hpp:149)
==29504== by 0x4E87B93: boost::asio::async_result<boost::asio::handler_type<curl::multi::Handle::Private::Timeout::Private::async_wait_for(std::weak_ptr<curl::multi::Handle::Private> const&, std::chrono::duration<long, std::rati...

Read more...

review: Needs Fixing
Revision history for this message
Michi Henning (michihenning) wrote :

Look, I'm starting to feel a bit guilty here. I don't want to hold up this fix unnecessarily. Most certainly, things are *heaps* better now than they were when I opened the bug.

But it's a concern to me that it doesn't run on my Utopic VM. I can build just fine, but the tests blow up. That, by itself, isn't that important to me. But, when I point my LD_LIBRARY_PATH at the libnet-cpp I have built an run the scopes-api unity tests, all the smartscopesproxy tests blow up too, and that *is* important to me :-)

So, on my Utopic machine, the net-cpp I got from the archives works fine, but the one I've built from source doesn't. There *has* to be some difference that matters. I don't have any PPAs installed on this machine, and my packages are all up to date. I'm really stumped on this one.

Would it be possible to instrument libcurl to tell us whatever it couldn't find? I haven't looked at the libcurl source yet. I might try building that and seeing whether I can insert some trace at the point of failure to tell me *what* it is that curl can't find. This has to be the most singularly useless error message I've seen in some time: "I couldn't find something or other. It's your job to guess what it is..." :-(

Revision history for this message
Michi Henning (michihenning) wrote :

Running ldd on the libnet-cpp.so in /usr/lib/x86_64-linux-gnu gives:

libcurl.so.4 => /usr/lib/x86_64-linux-gnu/libcurl.so.4 (0x00007f636c483000)

That's a symlink to /usr/lib/x86_64-linux-gnu/libcurl.so.4.3.0

Running ldd on the libnet-cpp.so that I've built myself gives:

libcurl-gnutls.so.4 => /usr/lib/x86_64-linux-gnu/libcurl-gnutls.so.4 (0x00007f4e64426000)

That's a symlink to /usr/lib/x86_64-linux-gnu/libcurl-gnutls.so.4.3.0

That would suggest that libcurl-gnutls.so.4.3.0 has a run-time dependency on something that isn't there?

I tried building with libcurl4-openssl-dev installed instead of libcurl4-gnutls-dev. Interestingly, with that, *only* the http_client_test fails whereas, with libcurl4-gnutls-dev, *both* the http_client_test and the http_client_load_test fail.

Revision history for this message
Michi Henning (michihenning) wrote :

So, summary:

with libcurl4-gnutls-dev installed, things break. With libcurl4-openssl-dev installed, things work. (The failure of the client_http_test in that case is due to not being able to connect to 127.0.0.1:5000 instead of a missing run-time dependency.)

Revision history for this message
Thomas Voß (thomas-voss) wrote :

> So, summary:
>
> with libcurl4-gnutls-dev installed, things break. With libcurl4-openssl-dev
> installed, things work. (The failure of the client_http_test in that case is
> due to not being able to connect to 127.0.0.1:5000 instead of a missing run-
> time dependency.)

Yup, but: the new debian/control setup in this MP takes care of replacing -gnutls with -openssl. So things should be good.

Revision history for this message
Michi Henning (michihenning) wrote :

 > Yup, but: the new debian/control setup in this MP takes care of replacing
> -gnutls with -openssl. So things should be good.

I'm not sure that's sufficient. The problem happened to me when I pulled the branch, typed "cmake", got a successful build, and then had the tests blow up with no sensible indication of what was wrong.

I think the cmake configuration step should check that the run-time dependencies are satisfied. Without this, the next unlucky person to come along and do what I did will get the same surprise.

From memory, I think the python stuff also has some run-time dependencies? These should be verified by cmake too.

lp:~thomas-voss/net-cpp/fix-1423765 updated
54. By Thomas Voß

Augment the exception thrown when setting the default ssl engine option with a more understandable descriptive text.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michi Henning (michihenning) wrote :

Thanks for all your work and persistence, Thomas!

review: Approve
Revision history for this message
Michi Henning (michihenning) wrote :

The failure on amd64 is the same one I got with my Utopic VM. It was caused by me not having python-flask installed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2014-06-27 08:16:12 +0000
3+++ debian/control 2015-03-03 11:12:49 +0000
4@@ -13,7 +13,6 @@
5 libboost-dev,
6 libboost-serialization-dev,
7 libboost-system-dev,
8- libcurl3,
9 libcurl4-openssl-dev,
10 libjsoncpp-dev,
11 libprocess-cpp-dev,
12
13=== modified file 'src/core/net/http/impl/curl/easy.cpp'
14--- src/core/net/http/impl/curl/easy.cpp 2014-05-06 07:57:16 +0000
15+++ src/core/net/http/impl/curl/easy.cpp 2015-03-03 11:12:49 +0000
16@@ -20,6 +20,8 @@
17
18 #include "shared.h"
19
20+#include <cassert>
21+
22 #include <atomic>
23 #include <chrono>
24 #include <condition_variable>
25@@ -37,12 +39,12 @@
26 {
27 Init()
28 {
29- curl::easy::throw_if_not<curl::Code::ok>(curl::native::init());
30+ curl::easy::throw_if_not<curl::Code::ok>(curl::easy::native::global::init());
31 }
32
33 ~Init()
34 {
35- curl::native::cleanup();
36+ curl::easy::native::global::cleanup();
37 }
38 } init;
39 }
40@@ -64,16 +66,21 @@
41 return result;
42 }
43
44-::curl::Code curl::native::init()
45+::curl::Code curl::easy::native::global::init()
46 {
47 return static_cast<::curl::Code>(curl_global_init(CURL_GLOBAL_DEFAULT));
48 }
49
50-void curl::native::cleanup()
51+void curl::easy::native::global::cleanup()
52 {
53 curl_global_cleanup();
54 }
55
56+std::string easy::print_error(curl::Code code)
57+{
58+ return std::string{curl_easy_strerror(static_cast<CURLcode>(code))};
59+}
60+
61 easy::native::Handle easy::native::init()
62 {
63 return curl_easy_init();
64@@ -146,8 +153,6 @@
65
66 std::shared_ptr<CURL> handle;
67
68- shared::Handle shared;
69-
70 easy::Handle::OnFinished on_finished_cb;
71 easy::Handle::OnProgress on_progress;
72 easy::Handle::OnReadData on_read_data_cb;
73@@ -155,6 +160,7 @@
74 easy::Handle::OnWriteHeader on_write_header_cb;
75
76 ::curl::StringList* header_string_list;
77+ char error[CURL_ERROR_SIZE];
78 };
79
80 int easy::Handle::progress_cb(void* data, double dltotal, double dlnow, double ultotal, double ulnow)
81@@ -221,9 +227,8 @@
82 easy::Handle::Handle() : d(new Private())
83 {
84 set_option(Option::http_auth, CURLAUTH_ANY);
85- set_option(Option::sharing, d->shared.native());
86+ set_option(Option::error_buffer, d->error);
87 set_option(Option::ssl_engine_default, easy::enable);
88-
89 set_option(Option::no_signal, easy::enable);
90 }
91
92@@ -423,7 +428,7 @@
93 void easy::Handle::perform()
94 {
95 if (!d) throw easy::Handle::HandleHasBeenAbandoned{};
96- throw_if_not<curl::Code::ok>(easy::native::perform(native()));
97+ throw_if_not<curl::Code::ok>(easy::native::perform(native()), [this]() { return std::string{d->error};});
98 }
99
100 // URL escapes the given input string.
101@@ -447,3 +452,8 @@
102 if (d->on_finished_cb)
103 d->on_finished_cb(code);
104 }
105+
106+std::string easy::Handle::error() const
107+{
108+ return std::string{d->error};
109+}
110
111=== modified file 'src/core/net/http/impl/curl/easy.h'
112--- src/core/net/http/impl/curl/easy.h 2014-05-06 11:05:04 +0000
113+++ src/core/net/http/impl/curl/easy.h 2015-03-03 11:12:49 +0000
114@@ -106,6 +106,7 @@
115
116 enum class Option
117 {
118+ error_buffer = CURLOPT_ERRORBUFFER,
119 cache_dns_timeout = CURLOPT_DNS_CACHE_TIMEOUT,
120 header_function = CURLOPT_HEADERFUNCTION,
121 header_data = CURLOPT_HEADERDATA,
122@@ -163,34 +164,40 @@
123 // Constant for enabling automatic SSL host verification.
124 constexpr static const long enable_ssl_host_verification = 2;
125
126+// Returns a human-readable description of the error code.
127+std::string print_error(Code code);
128+
129 // Throws a std::runtime_error if the parameter to the function does match the
130 // constant templated value.
131 template<Code ref>
132-inline void throw_if(Code code)
133+inline void throw_if(Code code, const std::function<std::string()>& descriptor = std::function<std::string()>())
134 {
135 if (code == ref)
136- {
137- std::stringstream ss; ss << code;
138- throw std::system_error(static_cast<int>(code), std::generic_category(), ss.str());
139- }
140+ throw std::runtime_error(print_error(code) + (descriptor ? ": " + descriptor() : ""));
141 }
142
143 // Throws a std::runtime_error if the parameter to the function does not match the
144 // constant templated value.
145 template<Code ref>
146-inline void throw_if_not(Code code)
147+inline void throw_if_not(Code code, const std::function<std::string()>& descriptor = std::function<std::string()>())
148 {
149 if (code != ref)
150- {
151- std::stringstream ss; ss << code;
152- throw std::system_error(static_cast<int>(code), std::generic_category(), ss.str());
153- }
154+ throw std::runtime_error(print_error(code) + (descriptor ? ": " + descriptor() : ""));
155 }
156
157 // All curl native types and functions go here.
158 namespace native
159 {
160-// An opaque handle to a curl multi instance.
161+// Global init/cleanup
162+namespace global
163+{
164+// Globally initializes curl.
165+Code init();
166+// Globally cleans up everything curl.
167+void cleanup();
168+}
169+
170+// An opaque handle to a curl easy instance.
171 typedef CURL* Handle;
172
173 // Creates and initializes a new native easy instance.
174@@ -282,7 +289,23 @@
175 template<typename T>
176 inline void set_option(Option option, T value)
177 {
178- throw_if_not<Code::ok>(native::set(native(), option, value));
179+ switch (option)
180+ {
181+ case Option::ssl_engine_default:
182+ throw_if_not<Code::ok>(native::set(native(), option, value), []()
183+ {
184+ return "We failed to setup the default SSL engine for CURL.\n"
185+ "This likely hints towards a broken CURL implementation.\n"
186+ "Please make sure that all the build-dependencies of the software are installed.\n";
187+ });
188+ break;
189+ default:
190+ throw_if_not<Code::ok>(native::set(native(), option, value), [this]()
191+ {
192+ return error();
193+ });
194+ break;
195+ }
196 }
197
198 // Adjusts the url that the instance should download.
199@@ -331,6 +354,9 @@
200 static std::size_t write_data_cb(char* data, size_t size, size_t nmemb, void* cookie);
201 static std::size_t write_header_cb(void* data, size_t size, size_t nmemb, void* cookie);
202
203+ // Returns the current error description.
204+ std::string error() const;
205+
206 struct Private;
207 std::shared_ptr<Private> d;
208 };
209
210=== modified file 'src/core/net/http/impl/curl/multi.cpp'
211--- src/core/net/http/impl/curl/multi.cpp 2014-03-18 12:33:18 +0000
212+++ src/core/net/http/impl/curl/multi.cpp 2015-03-03 11:12:49 +0000
213@@ -169,7 +169,7 @@
214 void cancel();
215
216 void async_wait_for(
217- const std::shared_ptr<multi::Handle::Private>& context,
218+ const std::weak_ptr<multi::Handle::Private>& context,
219 const std::chrono::milliseconds& ms);
220
221 void handle_timeout(const std::shared_ptr<multi::Handle::Private>& context);
222@@ -192,8 +192,8 @@
223 multi::native::Socket native);
224 ~Socket();
225
226- void async_wait_for_readable(const std::shared_ptr<Handle::Private>& context);
227- void async_wait_for_writeable(const std::shared_ptr<Handle::Private>& context);
228+ void async_wait_for_readable(const std::weak_ptr<Handle::Private>& context);
229+ void async_wait_for_writeable(const std::weak_ptr<Handle::Private>& context);
230
231 struct Private : public std::enable_shared_from_this<Private>
232 {
233@@ -202,8 +202,8 @@
234 ~Private();
235
236 void cancel();
237- void async_wait_for_readable(const std::shared_ptr<Handle::Private>& context);
238- void async_wait_for_writeable(std::shared_ptr<Handle::Private> context);
239+ void async_wait_for_readable(const std::weak_ptr<Handle::Private>& context);
240+ void async_wait_for_writeable(std::weak_ptr<Handle::Private> context);
241
242 bool cancel_requested;
243 boost::asio::posix::stream_descriptor sd;
244@@ -252,16 +252,21 @@
245 Accumulator for_start_transfer{};
246 Accumulator for_total{};
247 } accumulator;
248+
249+ struct Holder
250+ {
251+ std::weak_ptr<Private> value;
252+ } holder;
253 };
254
255 multi::Handle::Handle() : d(new Private())
256 {
257- auto holder = new Holder<Private>{d};
258+ d->holder.value = d;
259
260 set_option(Option::socket_function, Private::socket_callback);
261- set_option(Option::socket_data, holder);
262+ set_option(Option::socket_data, &d->holder);
263 set_option(Option::timer_function, Private::timer_callback);
264- set_option(Option::timer_data, holder);
265+ set_option(Option::timer_data, &d->holder);
266
267 set_option(Option::pipelining, easy::enable);
268 }
269@@ -375,23 +380,33 @@
270 timer.cancel();
271 }
272
273-void multi::Handle::Private::Timeout::Private::async_wait_for(const std::shared_ptr<Handle::Private>& context, const std::chrono::milliseconds& ms)
274+void multi::Handle::Private::Timeout::Private::async_wait_for(const std::weak_ptr<Handle::Private>& context, const std::chrono::milliseconds& ms)
275 {
276 if (ms.count() > 0)
277 {
278- auto self(shared_from_this());
279+ std::weak_ptr<Private> self{shared_from_this()};
280 timer.expires_from_now(boost::posix_time::milliseconds{ms.count()});
281- timer.async_wait([this, self, context](const boost::system::error_code& ec)
282+ timer.async_wait([self, context](const boost::system::error_code& ec)
283 {
284 if (ec)
285 return;
286
287- std::lock_guard<std::mutex> lg(context->guard);
288- handle_timeout(context);
289+ if (auto spc = context.lock())
290+ {
291+ if (auto sp = self.lock())
292+ {
293+ std::lock_guard<std::mutex> lg(spc->guard);
294+ sp->handle_timeout(spc);
295+ }
296+ }
297 });
298 } else if (ms.count() == 0)
299 {
300- handle_timeout(context);
301+ auto sp = context.lock();
302+ if (not sp)
303+ return;
304+
305+ handle_timeout(sp);
306 }
307 }
308
309@@ -411,12 +426,12 @@
310 if (timeout_ms < 0)
311 return -1;
312
313- auto holder = static_cast<Holder<Private>*>(cookie);
314+ auto holder = static_cast<Private::Holder*>(cookie);
315
316 if (!holder)
317 return 0;
318
319- auto thiz = holder->value;
320+ auto thiz = holder->value.lock();
321
322 thiz->timeout.async_wait_for(thiz, std::chrono::milliseconds{timeout_ms});
323
324@@ -434,12 +449,12 @@
325 d->cancel();
326 }
327
328-void multi::Handle::Private::Socket::async_wait_for_readable(const std::shared_ptr<multi::Handle::Private>& context)
329+void multi::Handle::Private::Socket::async_wait_for_readable(const std::weak_ptr<multi::Handle::Private>& context)
330 {
331 d->async_wait_for_readable(context);
332 }
333
334-void multi::Handle::Private::Socket::async_wait_for_writeable(const std::shared_ptr<multi::Handle::Private>& context)
335+void multi::Handle::Private::Socket::async_wait_for_writeable(const std::weak_ptr<multi::Handle::Private>& context)
336 {
337 d->async_wait_for_writeable(context);
338 }
339@@ -462,73 +477,81 @@
340 sd.release();
341 }
342
343-void multi::Handle::Private::Socket::Socket::Private::async_wait_for_readable(const std::shared_ptr<multi::Handle::Private>& context)
344+void multi::Handle::Private::Socket::Socket::Private::async_wait_for_readable(const std::weak_ptr<multi::Handle::Private>& context)
345 {
346- auto self(shared_from_this());
347- sd.async_read_some(boost::asio::null_buffers{}, [this, self, context](const boost::system::error_code& ec, std::size_t)
348+ std::weak_ptr<Private> self{shared_from_this()};
349+ sd.async_read_some(boost::asio::null_buffers{}, [self, context](const boost::system::error_code& ec, std::size_t)
350 {
351 if (ec == boost::asio::error::operation_aborted)
352 return;
353
354- if (cancel_requested)
355- return;
356-
357+ if (auto sp = self.lock())
358 {
359- std::lock_guard<std::mutex> lg(context->guard);
360-
361- int bitmask{0};
362-
363- if (ec)
364- bitmask = CURL_CSELECT_ERR;
365- else
366- bitmask = CURL_CSELECT_IN;
367-
368- auto result = multi::native::socket_action(context->handle, sd.native_handle(), bitmask);
369- multi::throw_if_not<multi::Code::ok>(result.first);
370-
371- context->process_multi_info();
372-
373- if (result.second <= 0)
374- context->timeout.cancel();
375+ if (sp->cancel_requested)
376+ return;
377+
378+ if (auto spc = context.lock())
379+ {
380+ std::lock_guard<std::mutex> lg(spc->guard);
381+
382+ int bitmask{0};
383+
384+ if (ec)
385+ bitmask = CURL_CSELECT_ERR;
386+ else
387+ bitmask = CURL_CSELECT_IN;
388+
389+ auto result = multi::native::socket_action(spc->handle, sp->sd.native_handle(), bitmask);
390+ multi::throw_if_not<multi::Code::ok>(result.first);
391+
392+ spc->process_multi_info();
393+
394+ if (result.second <= 0)
395+ spc->timeout.cancel();
396+
397+ // Restart
398+ sp->async_wait_for_readable(context);
399+ }
400 }
401-
402- // Restart
403- async_wait_for_readable(context);
404 });
405 }
406
407 void multi::Handle::Private::Socket::Socket::Private::async_wait_for_writeable(
408- std::shared_ptr<multi::Handle::Private> context)
409+ std::weak_ptr<multi::Handle::Private> context)
410 {
411- auto self(shared_from_this());
412- sd.async_write_some(boost::asio::null_buffers{}, [this, self, context](const boost::system::error_code& ec, std::size_t)
413+ std::weak_ptr<Private> self(shared_from_this());
414+ sd.async_write_some(boost::asio::null_buffers{}, [self, context](const boost::system::error_code& ec, std::size_t)
415 {
416 if (ec == boost::asio::error::operation_aborted)
417 return;
418
419- if (cancel_requested)
420- return;
421-
422+ if (auto sp = self.lock())
423 {
424- std::lock_guard<std::mutex> lg(context->guard);
425-
426- int bitmask{0};
427-
428- if (ec)
429- bitmask = CURL_CSELECT_ERR;
430- else
431- bitmask = CURL_CSELECT_OUT;
432-
433- auto result = multi::native::socket_action(context->handle, sd.native_handle(), bitmask);
434- multi::throw_if_not<multi::Code::ok>(result.first);
435-
436- context->process_multi_info();
437-
438- if (result.second <= 0)
439- context->timeout.cancel();
440+ if (sp->cancel_requested)
441+ return;
442+
443+ if (auto spc = context.lock())
444+ {
445+ std::lock_guard<std::mutex> lg(spc->guard);
446+
447+ int bitmask{0};
448+
449+ if (ec)
450+ bitmask = CURL_CSELECT_ERR;
451+ else
452+ bitmask = CURL_CSELECT_OUT;
453+
454+ auto result = multi::native::socket_action(spc->handle, sp->sd.native_handle(), bitmask);
455+ multi::throw_if_not<multi::Code::ok>(result.first);
456+
457+ spc->process_multi_info();
458+
459+ if (result.second <= 0)
460+ spc->timeout.cancel();
461+ }
462+ // Restart
463+ sp->async_wait_for_writeable(context);
464 }
465- // Restart
466- async_wait_for_writeable(context);
467 });
468 }
469
470@@ -541,12 +564,12 @@
471 {
472 static const int doc_tells_we_must_return_0{0};
473
474- auto holder = static_cast<Holder<Private>*>(cookie);
475+ auto holder = static_cast<Private::Holder*>(cookie);
476
477 if (!holder)
478 return doc_tells_we_must_return_0;
479
480- auto thiz = holder->value;
481+ auto thiz = holder->value.lock();
482 auto socket = static_cast<Socket*>(socket_cookie);
483
484 if (!socket)

Subscribers

People subscribed via source and target branches