Closed dok-net closed 1 year ago
I haven't considered something like OSAL yet but it should possible through a cmake option to disable certain parts of the code for compilation.
I would probably add a cmake option LIBCORO_OSAL which a user can enable when compiling the library. That in turn should set an add_definition to pass a preprocessor define into the code. Files that are not OSAL compatible should be wrapped in the define to disable compilation if the preprocessor OSAL define is present.
Are you still interesting in adding this here?
well, yes, but more like in a passive role where I would try to make it work after the basics have been introduced into the code already. If you like, please look at https://github.com/dok-net/libcoro/tree/minimal. Of course I would not want all the files that I have deleted to make it easier for me to determine a buildable subset to really be removed from the tree. But given existing mechanics in the CMakeLists, I would try to take it from there, probably first on the ESP8266. In the meantime, I have turned to https://github.com/dok-net/cpp-async.git (from Microsoft), because that is header-files only, very easy to integrate for POC, but unfortunately it leaks. If you would be able to take a little of your time and check https://github.com/microsoft/cpp-async/issues/1, perhaps your in-depth knowledge might make fixing the leak a one minute effort? I would like to think that it's more a matter of cooperation than competition, but I am good with either approach: my primary need right now is showing that using C++20 coroutines is possible and sane AT ALL :-)
In any case, thank you really a lot for caring and asking about my state in this matter, it means a lot.
I'm not sure I'll be able to find the memory leak any better on the cpp-async project :sweat_smile: this stuff its fairly complicated.
I'll take a look at what you excluded on your fork and see if it makes sense here as a specialized minimal build type.
I'm wondering if it makes more sense to mark this conditional compilation as enable networking or osal? It appears like on your fork you basically just removed all the networking classes.
I'm also curious because looking say here: https://github.com/nasa/osal/blob/main/docs/OSAL-Configuration-Guide.md they include a networking section. It seems like probably enabling networking as a feature is probably the right way to word this, and by default it is enabled?
It's not a one or the other exclusive choice, dropping all the networking stuff only seemed the logical first step to get anywhere. A straightforward way to adapt to any OS, for as long as C++ doesn't out of the box, which will be a long time coming still from judging how actively people are working on modern C++ support on the embedded OS options. Therefore, Some common prepared build system structure that allows to drop in wrappers around each OS's available primitives would mean a lot in terms of getting an overview of the effort, and not doing something that may end as no more than an experiment for my particular project. I am just assuming that you know what's really needed better than anyone else.
Can you take a look at the PR I posted? It could in theory be extended to other "features".
@jbaldwin I know it's asking a bit much, but... the particular thing I don't understand about MS's cpp-async is how there is no destroy() for the handle in their task<> impl. There is that helper class that has it, but that didn't help the leak. That said, are you certain that libcoro doesn't leak on the MCVE I posted over there?
I've never supported windows for this project, so I cannot answer that.
are you compiling it with the windows subsystem for linux, or not sure what thats called these days? Or am I completely misunderstanding what you are asking?
I mean, I think you nailed it looking at their code, the task
class doesn't have a destructor and the std::shared_ptr<task_state>
never calls the coroutines .destroy()
function, so I'm betting the coroutine frames are leaking. Adding a destructor to task
and explicitly calling the coroutines .destroy()
function might just solve your problem
@jbaldwin It's very nice of you that you are really trying to help. So, speaking about cpp-async, the great thing is that it's all headers only, which makes is so each to integrate into an Arduino sketch (ESP8266 Core for Arduino has a sufficient recent compiler, ESP32 with IDF 5.1 may soon, too). I'm getting consistent results of growing heap use on both Arduino and MS Visual Studio on Windows 11. As to libcoro, um, I haven't done much with it since asking for help here. I probably was building on WSL 2 Ubuntu, and may have attempted to build for i.MX RT1064 on Zephyr, obviously not completely because without porting that what I'd like the OSAL for just isn't there without it :-)
That makes sense, I posted what I think is probably the problem on the cpp-async project. I think it would be pretty easy for you to add the destructor in to test the theory.
@jbaldwin The networking part now closed this issue. I think it's not solved by that part alone. Would you consider re-opening?
I think I'd probably want an outline of what other parts should be enabled via feature flags?
All std::mutex uses. std::atomic, std::atomic_thread_fence, std::condition_variable, std::thread, std::jthread, and their supporting APIs. Also, compiling for targets without exceptions. I'm thinking of the facade pattern while writing this. try/catch is probably the ugly part in all this.
Hmm yeah removing exceptions might prove neigh impossible with stl usage in this library, I'm pretty sure I don't want to roll my own data structurres that underpin the coroutine constructs myself.
As far as I can tell, exceptions are properly used only for programming errors, not control flow. I could compile fine when I wrapped all throw, try, catch in #ifdefs. So all that's needed is a flag. The mutex etc should be approached in the way of OOP with the facade pattern, or any other that may come to your mind.
I don't think I'm going to pursue this much further since you've decided to use your own project, but I do like the conditional compilation we've added for some larger parts of the code, ssl being the main one to turn on/off.
TL;DR: Help needed in wrapping the C++ threading, synchronization, POSIX epoll, etc APIs in a new operating system abstraction layer for easy porting to different embedded OSes that have the features, but not the identical APIs.
I am pondering using libcoro on embedded systems, viz. Arduino and Zephyr BSPs. I haven't tried the comprehensive support for POSIX provided by NUTTX yet, given a recent compiler, it might be be possible to build for that OS with only minor tweaks. Anyway, the Zephyr SDK and at least the ESP8266 Arduino Core offer recent enough compiler toolchains to support.
What I am asking if there is any prior art or guidance for how to go about an OSAL in libcoro? Before I start reading all the sources :-) As first step, I have made a branch that (temporarily) purges all networking APIs and tests, but builds the remaining examples for Ubuntu. I should attempt building that for Zephyr native POSIX target in the coming days, if that works, it would be helpful in a step-by-step migration to an OSAL implementation.