wolkykim / libasyncd

Libasyncd is an embeddable event-based asynchronous Message/HTTP server framework for C/C++.
http://wolkykim.github.io/libasyncd/
Other
174 stars 36 forks source link

Uses Linux-specific APIs #14

Open codebrainz opened 9 years ago

codebrainz commented 9 years ago

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.

wolkykim commented 9 years ago

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?

codebrainz commented 9 years ago

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!

dascandy commented 8 years ago

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.

wolkykim commented 8 years ago

Sorry about that, yes the implementation is pretty much optimized to linux for now. But I'm very open to any porting ideas.

codebrainz commented 8 years ago

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).

spbruner commented 8 years ago

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...

wolkykim commented 8 years ago

is it only eventfd? if we make the eventfd part portable, will it be a go for the FreeBSD? we could make it portable.

spbruner commented 8 years ago

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.

codebrainz commented 8 years ago

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.

wolkykim commented 8 years ago

@spbruner your pull request was merged. Thank you. Please add yourself on the contributor list in the readme and send PR please.

spbruner commented 8 years ago

@codebrainz I believe you're probably correct in that assessment. I'll spend a little time this weekend experimenting with a libevent-only approach.

spbruner commented 8 years ago

@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.

codebrainz commented 8 years ago

Windows doesn't have eventfd or kqueue, so I think this issue is probably still valid.

spbruner commented 8 years ago

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.

manjunathkokhle commented 5 years ago

@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?