wahern / cqueues

Continuation Queues: Embeddable asynchronous networking, threading, and notification framework for Lua on Unix.
http://25thandclement.com/~william/projects/cqueues.html
MIT License
244 stars 37 forks source link

Build fails under illumos + epoll gaps? #257

Open faithanalog opened 6 months ago

faithanalog commented 6 months ago

Currently, this project fails to build on illumos:

gcc -O2 -std=gnu99 -fPIC -g -Wall -Wextra  -Wno-missing-field-initializers  -Wno-override-init -Wno-unused  -DLUA_COMPAT_APIINTCASTS -I/opt/pkg/include/lua-5.4 -D_REENTRANT -D_THREAD_SAFE -D_GNU_SOURCE -Usun -D_XPG4_2 -D__EXTENSIONS__  -DCOMPAT53_PREFIX=cqueues -DCQUEUES_VENDOR='"quae@daurnimator.com"' -DCQUEUES_VERSION=20200726L -DCQUEUES_COMMIT='"b0c2bc5979f008a39300b01a0dc3e967c6257be9"' -c -o /home/vi/z/cqueues2/src/5.4/cqueues.o /home/vi/z/cqueues2/src/cqueues.c
/home/vi/z/cqueues2/src/cqueues.c: In function 'kpoll_alert':
/home/vi/z/cqueues2/src/cqueues.c:688:18: warning: implicit declaration of function 'port_send' [-Wimplicit-function-declaration]
  688 |         if (0 != port_send(kp->fd, POLLIN, &kp->alert)) {
      |                  ^~~~~~~~~
/home/vi/z/cqueues2/src/cqueues.c: In function 'kpoll_isalert':
/home/vi/z/cqueues2/src/cqueues.c:762:21: error: 'kpoll_event_t' {aka 'const struct epoll_event'} has no member named 'portev_source'
  762 |         return event->portev_source == PORT_SOURCE_USER;
      |                     ^~
/home/vi/z/cqueues2/src/cqueues.c:762:40: error: 'PORT_SOURCE_USER' undeclared (first use in this function)
  762 |         return event->portev_source == PORT_SOURCE_USER;
      |                                        ^~~~~~~~~~~~~~~~
/home/vi/z/cqueues2/src/cqueues.c:762:40: note: each undeclared identifier is reported only once for each function it appears in
/home/vi/z/cqueues2/src/cqueues.c:766:1: warning: control reaches end of non-void function [-Wreturn-type]
  766 | } /* kpoll_isalert() */
      | ^

These errors appear to be because while ENABLE_EPOLL is defined, so is ENABLE_PORTS. But, because ENABLE_EPOLL is defined, ports.h is never included.

Now that said, I think that some of these port usages may be unintentional, since it seems like the intent is to use epoll instead of ports when available.

I was able to get the code to compile and work with epoll, with this patch, applied to b0c2bc5979f008a39300b01a0dc3e967c6257be9 (master as of writing):

diff --git a/src/cqueues.c b/src/cqueues.c
index c9d6299..2bd1e5d 100644
--- a/src/cqueues.c
+++ b/src/cqueues.c
@@ -478,7 +478,17 @@ static int kpoll_ctl(struct kpoll *, int, short *, short, void *);
 static int alert_rearm(struct kpoll *);

 static int alert_init(struct kpoll *kp) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+   int error;
+
+   if (kp->alert.fd[0] != -1)
+       return 0;
+
+   if ((error = cqs_pipe(kp->alert.fd, O_CLOEXEC|O_NONBLOCK)))
+       return error;
+
+   return alert_rearm(kp);
+#elif ENABLE_PORTS
    (void)kp;
    return 0;
 #elif HAVE_EVENTFD
@@ -503,7 +513,9 @@ static int alert_init(struct kpoll *kp) {
 } /* alert_init() */

 static void alert_destroy(struct kpoll *kp, int (*closefd)(int *, void *), void *cb_udata) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+   (void)kp;
+#elif ENABLE_PORTS
    (void)kp;
 #else
    for (size_t i = 0; i < countof(kp->alert.fd); i++)
@@ -512,7 +524,9 @@ static void alert_destroy(struct kpoll *kp, int (*closefd)(int *, void *), void
 } /* alert_destroy() */

 static int alert_rearm(struct kpoll *kp) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+   return kpoll_ctl(kp, kp->alert.fd[0], &kp->alert.state, POLLIN, &kp->alert);
+#elif ENABLE_PORTS
    return 0;
 #else
    return kpoll_ctl(kp, kp->alert.fd[0], &kp->alert.state, POLLIN, &kp->alert);
@@ -583,7 +597,10 @@ static inline short kpoll_pending(const kpoll_event_t *event) {

 static inline short kpoll_diff(const kpoll_event_t *event NOTUSED, short ostate NOTUSED) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+   // XXX is this ok?
+   return ostate;
+#elif ENABLE_PORTS
    /* Solaris Event Ports aren't persistent. */
    return 0;
 #else
@@ -684,7 +701,8 @@ static int kpoll_alert(struct kpoll *kp) {
    /* initialization may have been delayed */
    if ((error = alert_init(kp)))
        return error;
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+#elif ENABLE_PORTS
    if (0 != port_send(kp->fd, POLLIN, &kp->alert)) {
        if (errno != EBUSY)
            return errno;
@@ -720,7 +738,8 @@ static int kpoll_alert(struct kpoll *kp) {
 static int kpoll_calm(struct kpoll *kp) {
    int error;

-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+#elif ENABLE_PORTS
    /* each PORT_SOURCE_USER event is discrete */
 #elif HAVE_EVENTFD
    uint64_t n;
@@ -758,7 +777,9 @@ static int kpoll_calm(struct kpoll *kp) {

 static inline short kpoll_isalert(struct kpoll *kp, const kpoll_event_t *event) {
-#if ENABLE_PORTS
+#if ENABLE_EPOLL
+   return kpoll_udata(event) == &kp->alert;
+#elif ENABLE_PORTS
    return event->portev_source == PORT_SOURCE_USER;
 #else
    return kpoll_udata(event) == &kp->alert;

I do not actually know if I am doing things correctly, because I have never written code that uses epoll or ports directly. But this does make make lua-http from https://github.com/daurnimator/lua-http run correctly on my illumos machine (OmniOS-based distribution).

Alternatively, simply disabling EPOLL and using ports instead also worked:

diff --git a/src/cqueues.h b/src/cqueues.h
index ef803ea..d0c37a4 100644
--- a/src/cqueues.h
+++ b/src/cqueues.h
@@ -74,10 +74,6 @@

 #define UCLIBC_PREREQ(M, m, p) (defined __UCLIBC__ && (__UCLIBC_MAJOR__ > M || (__UCLIBC_MAJOR__ == M && __UCLIBC_MINOR__ > m) || (__UCLIBC_MAJOR__ == M && __UCLIBC_MINOR__ == m && __UCLIBC_SUBLEVEL__ >= p)))

-#ifndef ENABLE_EPOLL
-#define ENABLE_EPOLL HAVE_EPOLL_CREATE
-#endif
-
 #ifndef ENABLE_PORTS
 #define ENABLE_PORTS HAVE_PORT_CREATE
 #endif
daurnimator commented 6 months ago

Now that said, I think that some of these port usages may be unintentional, since it seems like the intent is to use epoll instead of ports when available.

At least when we wrote it, there was no epoll on illumos and ports was the only option. I don't think we ever considered a target/day when there would be both.