- It's containing it's own abstraction over pthread instead of std::thread/std::mutex/...
- It passes shared_ptr via reference (yeah, this saves an increment and decrement on the refcounting, but the code is not tuned that much)
- it prints directly to sterr (via fprintf) making it hard to redirect errors somewhere else
- It seems not to be prepared to be ported to other platforms
- It could make use of std::chrono instead of int64 timestamps
- no build system (no CMake or anything)
- tests not integrated, look verbose, all tests individually recompile all files, makes it hard to work test-drive
In summary I see no compelling reason over boost asio or such and potential issues.
I agree with the last sentence. Nevertheless, if it's mainly a learning project for the author, I don't think there's anything wrong with writing it.
Some technical review for the presented library:
- It doesn't seem to support any kind of backpressure, which is basically a nogo if you want to build something reliable on top. On the receiving side you will always get data pushed via callbacks without the possibility to stop it. On the sending side it will always return void and queue the data internally if it can't be send immediatly. A slow, malfunctioning or attacking remote can DOS you through that. Ways to implement backpressure are pull-style operations like in boost asio (you start a read and get a single callback when it's done) or some pause/unpause functions and buffer treshold indicators (like in node.js). I personally prefer the first model now, especially if the single operations return something like a promise.
- That brings me to the next point: The framework does not seem to have a good way to build composable operations. E.g. build a function that reads a websocket frame from the socket. Or another function that performs the websocket handshake by reading the HTTP handshake request and sending the associated response, but leaving the stream intact for any following user to to be able to use if for sending websocket frames. And ideally all operations should be trivially boundable by timeouts. With having a single receive callback for available data there's mostly a need for complex callback-driven state machines.
- Imho a new good framework should also provide a sophisticated and universal story for cancellation of composed operations. Like backpressure that's a thing which can be avoided for demo applications, but for battleproof production environments it is necessary.
- Thread safety and data races: There seem to be a few issues, e.g. TcpConnection::state isn't synchronized and used from multiple threads. loop_->runInLoop([&](){ this->shutdownInLoop(); }); will segfault or cause undefined behavior if the TcpConnection object was deleted befor the queued functor runs. There also might be some reentrancy issues: In each callback to the user (like MessageCallback) the user might fiddle around with the object and change it's internal state. If that isn't expected and guarded against then code that runs after the invocation of the callback might not work as intended. By the way: That's the situation where js-style promises shine: As the callbacks are not immediatly invoked but in the next eventloop iteration there is less rooom for errors. But of course it costs additional performance.
If these things now sound as a harsh critique let me try to bring it back into relation again. First of all: Network programming is super hard because of "concurrency everywhere". There are always some execution paths that one hadn't thought about before, and it takes a lot of time to learn all the gotchas. I do that stuff now for 8 years, probably had worse assumptions and code for quite some time, and still learn now things every day. It's for sure not a bad idea to write an own library in order to learn these things. For users the well-known libraries (asio, libuv, QT Network, etc.) might be a more solid choice.
Also in my opinion even the big and well-known libraries have some dark corners, there hasn't been a golden solution to network programming yet:
- asio is hard to use, callback that are happening on invalidated objects or operations that are still running and use invalidated buffers are are common problem for non-experts. It get's worse if you use asio with multiple threads (especially one io_service and multiple threads). Composed operations are possible but hard to write efficiently (best put all shared-state between operations in shared_ptr's and write a state machine. Maybe it's better with coroutine support.
- libuv is generally well engineered, but is still doesn't offer lots of support for higher level composed operations and cancellation.
- Netty has some dark corners to watch out for. E.g. if handlers are exchanged during runtime (e.g. for websocket upgrades) or handlers are running in multiple threads there's quite some room for errors on the user side. Otherwise it's a well engineered framework in many areas.
- node.js stream abstractions with their multiple modes can be tricky to understand and to use for higher-level abstractions. I guess if networking would be based on promises with async/await support it would be easier now - but those weren't available back then.
Imho the most promising concepts for network programming are the coroutine based ones. Go's model works well, is on the easier side to use (even though goroutines/threads introduce more possibilities for race conditions). It ticks most boxes, e.g. backpressure is naturally available through sync APIs. Cancellation could probably be improved.
Martin Sustriks experiments with libdill also look interesting. As well as Kotlins coroutine model.
The promise based models with async/await are also quite promising, e.g. in C# or node.js. However I find those models fall a little bit down as soon as they go to support both multiple threads and an event loop. Having to remember which callbacks/continuations run on which thread and what is allowed there is cumbersome. Imho either multiple threads with synchronous code or a single thread with an eventloop is ok.
Ooops, that was now a little bit more text than I intended to write. But maybe it helps someone.
Recently modified a legacy C++ program at work that was using 'bare' socket code, that now needed to support multiple connections. First approach using Asio worked, but as you mentioned involved scattering logic in various callbacks and transferring state around everywhere. And that was just the 'pure' connection establishment code; the business logic part of things also needed to occasionally do comm, and it wasn't really viable to rip up the legacy call hierarchy to convert things into callback form.
Enter Asio + stackful coroutines. Migrating the old business logic was nearly effortless. Just had to be wary of accidentally blowing the stack, and watch for exceptions in the various coroutines' mains. But otherwise able to preserve all that old, functioning code while meeting the new requirements.
Which coroutine library did you went for? Boost fiber? I want to explore that at some time, but couldn't use it in my main asio project since that requires support for older boost versions and ideally also for obscurer platforms.
Good to hear that you had a good experience with that approach.
Can you explain what you mean here? Isn't what the op is doing generally ok - to pass shared_ptr's by const reference to save an additional increment/decrement on function call?
It isn't idiomatic. If you've already locked the shared_ptr, you should extract it's boxed value and pass that by reference directly. Passing a shared_ptr by reference has fairly niche applications and allows the callee to do things like reset the shared_ptr, extract a weak_ptr from it, etc.
That said, if you are in need of the latter use case, it certainly should be passed by reference. It's not just a reference count! Shared pointers in C++ are threadsafe so there's a fair bit more going on under the hood that makes copying it (more) expensive.
C/C++ doesn't really have good general networking libraries that fit all purposes. On that note, I say: yes, great topic!
However.
Portability is one of the key features of C++, and why people still choose it for new projects. If one can constrain project to Linux I'm sure the project constraints would allow a more productive language to be used altogether.
On the one hand, you're right -- in that I can see and respect why you say that.
On the otherhand, what the heck is wrong with using the syscalls? Every time I write a new high performance networking application, sure, i lose a day or two building primitives out of send recvmsg listen accept and splice, but once i'm done, i'm done, and things work.
I'm confused why people feel the need to make paper thin abstraction layers covering the berkeley sockets API. In the time it takes one to learn how to use the flavor of the month, one probably has gotten halfway there to understanding how to Really do networking.
It's not like networking on Linux is even a shred harder than easy...mumbles off into the distance unlike BLE on Windows10 (and linux)....
Tangentially, I positively can't WAIT for Networking support in c++20 -- that and Concepts I've been waiting for with baited breath.
[EDIT: apologies, i think my tone got away with me.. stupid aspie tendencies... what i MEANT to say somewhere in there, directed to the author, was: "Good Job!!" It's hard work to put yourself out there and enjoy accomplishment. Please do let the rest of the community know if you'd welcome help in porting it to other platforms and adding features and improving performance. Best!]
> On the one hand, you're right -- in that I can see and respect why you say that.
> On the otherhand, what the heck is wrong with using the syscalls? Every time I write a new high performance networking application, sure, i lose a day or two building primitives out of send recvmsg listen accept and splice, but once i'm done, i'm done, and things work.
...
> Tangentially, I positively can't WAIT for Networking support in c++20 -- that and Concepts I've been waiting for with baited breath.
You know that the networking support in c++20 is just Boost.Asio with namespace std, right ? You could have been using the exact same API today (or ten years ago for what it's worth), which is header-only, and supports TCP, UDP, Unix sockets, SSL, serial port communication, and all of this either synchronously or asynchronously, etc...
> I'm confused why people feel the need to make paper thin abstraction layers covering the berkeley sockets API.
* that's like saying "std:string is a thin abstraction layer over const char* and strlen". There is much more to this in big networking libraries: event loop handling, thread pools, etc.
It would be great if there was an obvious example of some sort in the README. I really have no idea what this offers other than “Linux-only” and “async”
Given the universality of C++, everyone effectively ends up writing in their own dialect and there is a sizeable faction that uses it as "C with Classes" and "C with templates". So there is a lot of C++ projects that explicitly steer clear of modern C++ features, especially derived from the mother of all unholy abominations and the manifestation of everything that is wrong with the modern C++, the boost library.
They are also the projects that get the most attention. I wouldn't rule out that python/javascript/whatever projects have just as many, if not even more, security flaws than your typical widely-used C library. They are just not interesting enough to warrant the required attention. The flaws are likely to be more subtle than simple buffer overflows too, think of all the abstractions involved.
... as are the projects that overdo on "modern C++" to the extent of introducing even more severe problems masked by the layers of needless abstraction.
CVEs don't come from C or C++. They come from the lack of coding discipline and with all other things being equal C programmers are generally better skilled and more diligent than their C++ counterparts.
One would expect, on a C project with the rigor of review process that patches go through and the usual assement I keep hearing since the early 90's about C programmer skills, now repeated again by you, that Linux would have 0 CVEs reported.
The majority of exploits in C++ are related to its copy-paste compatibility with C, and devs that insist in using them in spite of the language offering safer, better, alternatives.
Exactly because it is "largest, complex and most scrutinized C projects of the whole time" and yet it is impossible to use C without producing a couple of CVEs every single month.
And a good example against the myth that the skilled C programmer that doens't produce memory corruption exists.
Again - what are you comparing Linux CVE counts to?
You can't point at an absolute count for A while arguing that A is inferior to B. I see nothing to support the assumption that a Linux-sized project in C++ would've have any less of CVEs.
> the myth that the skilled C programmer that doens't produce memory corruption exists.
Don't put words in my mouth. You are refuting a point that I didn't make. I said that C programmers are generally better skilled than C++ ones, which is not a "myth".
> I said that C programmers are generally better skilled than C++ ones, which is not a "myth".
So where are those statistic results available?
Every single memory corruption in C++ code can be traced back at language semantics inherited from C, thus common to both languages.
Which aren't present in C++ code bases where developers adopt C++ best practices, meaning eschewing the majority of C programming pattern, while adopting std library types, references instead of pointers, class enumerations instead of plain enums, classes to keep struct invariants, ....
History has proven on its CVE databases that the majority of C devs are not skilled at all to care about writing safe code.
Whereas one of the major points done by ANSI C++ members work is how to reduce UB use cases and reduce the possibilities of exploits, the ANSI C members decided to turn ANNEX K into an optional item in C11, and VLAs have been proven to be quite nice for stack attacks, thus not even considered for ANSI C++ adoption.
I used boost extensively in the past (2006-2011). But I have been disillusioned by all the accidental complexity. It's easy to use the complicated parts of boost, but you lose sight of the inefficiencies you introduce all over the place, be it code bloat, unnecessary indirections, splitting up of allocations, or merely bloating your build times.
Besides, std::unordered_map is flawed, the bucket interface forces suboptimal cache behavior. std::thread is such a thin abstraction, if you use C++ you might as well use it but it doesn't buy you much. same with std::mutex. std::regex on the other hand I avoid because it's easy to write "regular" expressions that cause backtracking with it. I would look at something like re2 instead, if I had to deal with text parsing a lot. or a proper LALR parser generator if the language is not regular.
Once they're safely out of Boost and implemented by the stdlib they're an order of magnitude more convenient to use. You can also be more confident in their future stability.
> Once they're safely out of Boost and implemented by the stdlib they're an order of magnitude more convenient to use.
I don't understand this. Most of the time the API doesn't even change; you can just do `namespace lib = std` or `namespace lib = boost` according to which one you want to use. In my own code for instance I have a header where I choose the std::, std::experimental or boost:: version of `optional`, `string_view`, and `any` according to the recentness of the stdlib I'm using; nothing else is needed.
had a quick look at the src/tests, from what I can tell, it is more like a hobby project started for learning c++11/networking. a few of those tests are really more like examples
- It's containing it's own abstraction over pthread instead of std::thread/std::mutex/... - It passes shared_ptr via reference (yeah, this saves an increment and decrement on the refcounting, but the code is not tuned that much) - it prints directly to sterr (via fprintf) making it hard to redirect errors somewhere else - It seems not to be prepared to be ported to other platforms - It could make use of std::chrono instead of int64 timestamps - no build system (no CMake or anything) - tests not integrated, look verbose, all tests individually recompile all files, makes it hard to work test-drive
In summary I see no compelling reason over boost asio or such and potential issues.