Uses Linux-specific APIs
Opened this issue · 15 comments
Looking at the source, it unconditionally includes <sys/eventfd.h> and uses the (really nice) eventfd API. This means libasyncd will only run on Linux (and possibly some BSDs if they can emulate it). I'm not very familiar with libevent, but it might provide some portable replacement for the use of eventfd.
That is right. What O/S are you using? It's written to work best on Linux but if there's needs we can always do the porting work. I guess the question is are there the needs?
It's fine for now to support only Linux, I just didn't realize it at first, and I assumed it used some semi-portable (ie. POSIX) stuff. It might be worth to add a note in the README.md like "Currently only supports Linux but cross-platform improvements welcome" or something.
I probably won't use this library at the moment due to my project not being very important and kind of wanting cross-platform support out-of-the box, but it seems to be pretty interesting still, so I might try to contribute a bit, at least with the Autotools.
Nice work!
I was going to create an issue that it does not cleanly compile on OS X but this is pretty much exactly that issue. The concrete things I see is that you use "-Wl,-soname" which does not work on OSX's linker, and you include sys/eventfd.h which does not exist.
Also, there's the potential for Windows support but I am not sure how many libraries would need porting before that would work - I stopped counting when libevent didn't work.
Sorry about that, yes the implementation is pretty much optimized to linux for now. But I'm very open to any porting ideas.
Maybe using a pipe or semaphore to replace eventfd. Windows has a whole different mechanism for async IO than Unix, but I think if libasyncd could stick to POSIX APIs, it could support most Unix out-of-the-box, and would make porting to Windows somewhat easier (Win32 has select() and WSAPoll() but I'm not sure if they work on non-socket handles).
I'd originally written a comment here that said it appears you can safely remove the eventfd.h include, but I was mistaken. A second read through of ad_server.c does indeed indicate usage of the eventfd API.
So, sadly, no-go for now.
Now to find a way to generalize for the sake of portability...
is it only eventfd? if we make the eventfd part portable, will it be a go for the FreeBSD? we could make it portable.
Well, most of your library is built on libevent, so yes I think its just the eventfd function. At least that I've seen so far. In my testing on FreeBSD, if I replace the single call to eventfd(0,0) with kqueue() the library builds and runs as you'd expect.
As to your question whether it could be made more portable, I don't really see you getting around the fact that different OS's have different event notification interfaces. I don't really see a problem with that either. In my own repository, I added a quick and dirty #ifdef to use eventfd on Linux, and kqueue otherwise (which covers the "big three" BSD's as well as Mac OS). Long term, I'd recommend adding an ad_platform (a name I pulled out of thin air) header with a get_descriptor (or similar) function that abstracts away the platform-dependent interface. The idea being you have a single place where you can place all those dirty #ifdef's rather than polluting the core C files with them.
That's just my two cents.
If it uses libevent, there's probably not really a need to use any platform specific stuff. AFAIK libevent already contains the needed platform abstractions. A quick skim of the API docs, maybe the eventfd notification channel part could be replaced with event_new passing -1 for the file descriptor and 0 for the flags.
@spbruner your pull request was merged. Thank you. Please add yourself on the contributor list in the readme and send PR please.
@codebrainz I believe you're probably correct in that assessment. I'll spend a little time this weekend experimenting with a libevent-only approach.
@wolkykim You can probably go ahead and close this issue. While we may continue to search for a better way or just cleaner code, we have at least one working mechanism that directly addresses this issue.
If anyone on thread is using Mac OS X, I'd appreciate if you could test it and drop me a quick note with your findings.
Windows doesn't have eventfd or kqueue, so I think this issue is probably still valid.
Ok. Well I really couldn't find a way to create a file descriptor using libevent alone. So I went with my original plan and refactored my previous addition into the afore-mentioned ad_platform(.h/.c) and swapped out the call in ad_server.c to call the new ad_patform_getfd() function. I'm testing this change now and will likely upload with a PR later today.
@codebrainz Windows has IO Completion Ports, which is about as close as you'll get. But since its been 10+ years since I've done any Win32 programming, I'll leave that to someone else to experiment with and add as needed.
@codebrainz / @wolkykim
i am compiling the code for the embedded environment where i dont have eventfd. though i have managed to get through the error by commenting the EVENT__HAVE_EVENTFD flag but i am stuck in eventfd function call in ad_server_start(). is there a alternate method to call eventfd for linux environment using pipe?