Merge lp:~thomas-voss/net-cpp/fix-1423765 into lp:net-cpp
- fix-1423765
- Merge into trunk
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 | ||||||||
Related bugs: |
|
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.
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.
- 47. By Thomas Voß
-
Disable sharing to avoid memory leaks.
Thomas Voß (thomas-voss) wrote : | # |
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-
==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_
==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/
==30468== by 0x4E8FDDD: boost::
==30468== by 0x4E8D6AA: boost::
==30468== by 0x4E8985A: boost::
==30468== by 0x4E88D99: boost::
==30468== by 0x4E8BE6C: boost::
==30468== by 0x4E8C1B6: boost::
==30468== by 0x4E922D2: boost::
==30468== by 0x4E8F84E: boost::
==30468== by 0x4E83FC9: curl::multi:
==30468== by 0x4E83E4E: curl::multi:
==30468== by 0x4E84741: curl::multi:
==30468==
=...
- 48. By Thomas Voß
-
Adjust build- and runtime deps for curl.
Michi Henning (michihenning) wrote : | # |
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/
==13111== by 0x4E878E7: boost::
==13111== by 0x4E879F6: boost::
==13111== by 0x4E855F0: void* boost_asio_
==13111== by 0x4E8513C: void boost::
==13111== by 0x4E84E71: boost::
==13111== by 0x4E84D63: boost::
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?
- 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.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:53
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Michi Henning (michihenning) wrote : | # |
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/
==29504== by 0x4E8A699: boost::
==29504== by 0x4E8A7A8: boost::
==29504== by 0x4E883DE: void* boost_asio_
==29504== by 0x4E87F56: void boost::
==29504== by 0x4E87CA1: boost::
==29504== by 0x4E87B93: boost::
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..." :-(
Michi Henning (michihenning) wrote : | # |
Running ldd on the libnet-cpp.so in /usr/lib/
libcurl.so.4 => /usr/lib/
That's a symlink to /usr/lib/
Running ldd on the libnet-cpp.so that I've built myself gives:
libcurl-gnutls.so.4 => /usr/lib/
That's a symlink to /usr/lib/
That would suggest that libcurl-
I tried building with libcurl4-
Michi Henning (michihenning) wrote : | # |
So, summary:
with libcurl4-gnutls-dev installed, things break. With libcurl4-
Thomas Voß (thomas-voss) wrote : | # |
> So, summary:
>
> with libcurl4-gnutls-dev installed, things break. With libcurl4-
> 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.
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.
- 54. By Thomas Voß
-
Augment the exception thrown when setting the default ssl engine option with a more understandable descriptive text.
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:54
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Michi Henning (michihenning) wrote : | # |
Thanks for all your work and persistence, Thomas!
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
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) |
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 :-(