Timeline for C++ Wrapper for cURL: Multithreading and serializing asynchronous ops
Current License: CC BY-SA 3.0
21 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Jul 6, 2019 at 15:51 | comment | added | Loki Astari | @quetzalcoatl Where it written in C++ it would not use techniques that made it brittle. :-) | |
| Jul 6, 2019 at 11:01 | comment | added | quetzalcoatl | @MartinYork: you're right, I forgot that curl_easy_setopt is itself written in C, not C++. Were it C++, it'd be safe to pass a static member function. | |
| Jul 5, 2019 at 17:21 | comment | added | Loki Astari | @quetzalcoatl Looking at your link it does not provide any gurantees about the ABI across different languages. Only within C++ itself (which is a different language to C). Which is why you have the "extern C" declaration so that on compilers where there is a difference the appropriate code can be generated. | |
| Jul 5, 2019 at 17:18 | comment | added | Loki Astari |
@quetzalcoatl: It may work on some platforms. The problem is there is no ABI definition for C++ functions so you can not guarantee that a static member function has the same ABI as an extern "C" function. libcurl is a C library and is built to accept extern "C" functions so it will always work with C functions. But you can not guarantee it will work with C++ functions (even static class members). Because you can not make that guarantee it is UB. Now UB does not mean it will not work it just means you have to test (and re-test every time the compiler changes).
|
|
| Jul 5, 2019 at 15:22 | comment | added | quetzalcoatl |
Why do you claim that curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, &CppUrl::writeCallback); is an UB? writeCallback is static! see for example stackoverflow.com/a/30949326/717732 - there's even citation from the standard indicating that static member functions are ordinary functions. Do you have anything to support your judgement? If not, I suppose you're wrong in that one point.
|
|
| Jan 10, 2017 at 3:54 | vote | accept | mr5 | ||
| Jan 10, 2017 at 3:53 | comment | added | mr5 | Oh Gawd! One of reasons I wanted this code reviewed is I will integrate this in my next project. Judging from our discussion, I guess I still have a lot of work to do in this library before proceeding to my next project. Again, thanks for your effort and patience for replying to my comment. I really appreciate it! I think that's enough. Have a nice day or have a good sleep! :) | |
| Jan 10, 2017 at 3:46 | comment | added | Loki Astari |
@mr5: So it's enough to say that the thread is asynchronous when I didn't invoke the std::thread::join()`. Yes. The thread is already running. When you call join it waits for the thread to finish (but it may have already finished).
|
|
| Jan 10, 2017 at 3:44 | comment | added | Loki Astari |
@mr5: Yes waitTask() is called before every cleanTask(). But you are artifically imposing a constraint on the code that is not documented. So a future maintainer MUST know this or he could accidently break the code. Why not make the task explicit. If cleanTask() always calls waitTask() then a maintainer can not get it wrong.
|
|
| Jan 10, 2017 at 3:44 | comment | added | mr5 |
My bad. I thought delete works only on ctor, dtor, cctor only :)
|
|
| Jan 10, 2017 at 3:42 | comment | added | mr5 |
So it's enough to say that the thread is asynchronous when I didn't invoke the std::thread::join() function?
|
|
| Jan 10, 2017 at 3:42 | comment | added | Loki Astari |
@mr5: might have destroyed first before the underlying std::thread. It better not. If the thread tries to use the object after the object is destroyed you will invoke nasal dragons. It is your responcability to make sure this never happens by making the destructor wait for the thread to finish before allowing the destructor to exit (another reason not to use detach()).
|
|
| Jan 10, 2017 at 3:39 | comment | added | Loki Astari | Your assignment operator is still valid and the code will compile if you use it. It may not link (because there is no implementation). But that is a very obscure error. Also why have one function removed using C++11 technique and one removed using a C++03 technique. Mark both as deleted. | |
| Jan 10, 2017 at 3:36 | comment | added | Loki Astari |
@mr5: what is the proper way to do an asynchronous work in C++. First detach() has nothing to do with async work, the thread is already running when you detach. In other languages when you call async(func) it creates something that looks like a thread that runs in the background. In your C++ you do this when you call new std::thread(func) you have already created the same effect; but I would argue that you should probably call std::async(func) in C++ as well (let C++ handle the thread for you).
|
|
| Jan 10, 2017 at 3:30 | comment | added | Loki Astari |
@mr5: curl_global_init: Added example to show it happening.
|
|
| Jan 10, 2017 at 3:30 | history | edited | Loki Astari | CC BY-SA 3.0 |
added 446 characters in body
|
| Jan 10, 2017 at 3:03 | comment | added | mr5 | All in all, thanks for pointing out my flawed design. I just started learning C++ again so somethings are not still clear for me. | |
| Jan 10, 2017 at 3:03 | comment | added | mr5 |
4. Both assignment operator and copy constructor are deleted? Remember, it's a pure header only so there's no implementation of assignment operator. 5. handle = nullptr I did this because I thought an instance of CppUrl might have destroyed first before the underlying std::thread when the user uses an async() function. 6. According to official docs, I should use curl_easy_reset if I wanted to reuse my handle and I think there's no harm if the handle wasn't used before. 7. waitTask() is called before every cleanTask()
|
|
| Jan 10, 2017 at 2:50 | comment | added | mr5 |
Hi! Sorry for late reply. I just like to clarify some things why I did those, and ask questions altogether: 1. So if I should not be using the std::thread::detach() what is the proper way to do an asynchronous work in C++? 2. I wanted to have a generic exception class, so the name itself MissingHttpMethodException should be enough for the programmer to know what he's missing. You're right, I should be using assert rather than exceptions here. 3. curlInstanceCount is a static variable, and it's not invoking curl_global_init multiple times, base from my tests.
|
|
| Jan 9, 2017 at 20:31 | history | edited | Loki Astari | CC BY-SA 3.0 |
deleted 119 characters in body
|
| Jan 9, 2017 at 20:24 | history | answered | Loki Astari | CC BY-SA 3.0 |