I've been reading the Corosio documentation, but it looks like I won't have time to finish reading the documentation much less thoroughly review the library before the end of the review period, even with the extension. So here's some quick thoughts. In dark mode, the documentation has black text on dark blue background that's very difficult to read. Even before getting to the Reference section, there is a lot of duplicate information in the documentation. The Guide > TCP/IP Networking section is already covered by the Networking Tutorial section above. Guide > TLS Encryption and Tutorials > TLS Context Configuration cover basically the same subject matter twice. The following is dangerously misleading: https://develop.corosio.cpp.al/corosio/2.networking-tutorial/2g.udp.html:
When a UDP datagram exceeds the path MTU, IP fragments it into smaller pieces. These fragments travel independently through the network. If every fragment arrives, the destination reassembles the original datagram and delivers it to your application. If any fragment is lost, the entire datagram is discarded. Your application receives nothing — not even the fragments that did arrive.
Yes, both IPv4 and IPv6 allow the receiver to reassemble fragmented packets, but they also allow the receiver to silently discard fragmented packets above a certain size, and that size limit is a mere 576 bytes for IPv4. That means that fragmented datagrams above that limit are not just a performance issue, but potentially a hard failure. Tutorials > TLS Context Configuration is full of red warning boxes that parts of the API are not yet wired up. That's understandable for a library that's still in development, but it suggests that the library is not ready for inclusion in Boost. Worse, it seems that Corosio does not fail safe where TLS is concerned: when the user program tries to use a TLS feature that is not yet implemented, Corosio still makes a TLS connection while silently not using that feature instead of (safely) refusing to connect. In Tutorials > Hash Server, the compute_fnv1a function should be a plain function, not a coroutine. It uses no coroutine functionality and can reasonably be reused in a non-coroutine context. It's clearly written as a coroutine to allow convenient use with capy::run without introducing an extra lambda, but that's still bad practice. Named functions should be written to be reusable where possible. The documentation is full of coroutines that take arguments by reference or view. That's asking for dangling references. I haven't actually seen any dangling references in the documentation, but the danger is obviously there. I see three possible strategies for reference arguments in coroutines: 1. Coroutines do not take arguments by references or view, so all coroutines are safe for use with run_async by default. 2. Coroutines are assumed to potentially take arguments by reference or view, so they must be wrapped at the point where run_async is called: capy::task<> f(std::string_view); void run_f_async(std::string_view s) { capy::run_async(ex)([inner_s = std::string(s)] -> capy::task<> { co_await f(inner_s); }); } 3. For toy examples only, call capy::run_async, pass data into the coroutine by reference or view, and make sure the data being references stays alive so long as the coroutine can potentially run. The examples in the Corosio documentation often use option 3. Option 3 is only suitable for toy examples. (The dangling reference problem is in fact acknowledged by the documentation, Guide > Concurrent Programming > Dangling References in Async Code, but the examples get dangerously close to ignoring it.) In Networking Tutorial > Opening and Closing TCP Connections, the state machine is presented as a linear list of states without connections. This would be acceptable if there were one linear path through the state machine, from top to bottom, but that's not the case because the path through the state machine splits based on which side of the connection closes the connection first, taking either the FIN_WAIT_1 -> FIN_WAIT_2 path or the CLOSE_WAIT -> LAST_ACK path. Why is Quick Start near the bottom of the documentation, between Glossary and Reference? corosio::io_context mixes the functionalities of waiting for i/o and running waiting coroutines when the i/o is available. It would be cleaner to separate this functionality: coroutines run in capy::thread_pool or any other executor and only call into corosio::io_context when they want to read or write data. I assume that the reason for not going with the cleaner separation of concerns is performance. Is so, fair enough, but this rationale should be documented. Thread safety is clearly documented at the class level, which is good. Executor affinity, which is just as important for correct concurrency, is not explicitly documented. I noticed that there's no generic ip_address type capable of holding either an IPv4 or an IPv6 address. The closest equivalent is corosio::endpoint, which also hold a port and is therefore not a suitable replacement. The Reference section would be more readable of it was grouped by functionality instead of throwing all types into a big heap and sorting it alphabetically. For example, there is no overview of the backend tags available, and the operators should be documented with the types they operate on, not as freestanding non-member functions. The Friends section for the individual types in the Reference section kind of does document which operators are available for which type, but it's the wrong way to do this for two reasons: - It excludes non-friend operators, like operator!= when it is defined in terms of operator==. - The fact that the implementation of an operator depends on a friend declaration is an implementation detail. It might be better to separate the synchronous functions from the asynchronous coroutines in the Reference sections so that one can see which operations are asynchronous. On a more positive note, I like how you can directly get and set the size of a random_access_file with a single function call. -- Rainer Deyke - rainerd@eldwood.com