jow- / ucode

JavaScript-like language with optional templating
ISC License
87 stars 24 forks source link

ubus: fix uc_ubus_have_uloop for eloop+uloop combination #198

Closed nbd168 closed 2 months ago

nbd168 commented 2 months ago

When uloop has been integrated with eloop (in hostapd/wpa_supplicant), uloop_cancelling will return false, since uloop_run is not being called. This leads to ubus.defer() calling uloop_run in a context where it can prevent the other event loop from running.

Fix this by detecting event loop integration via uloop_fd_set_cb being set

nbd168 commented 2 months ago

The failed CI run looks like outdated libubox to me

jow- commented 2 months ago

Can you add the following changes as well? This should fix CI and ensure that older versions continue to build.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1ffcadc..adc04ec 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -159,6 +159,7 @@ if(UBUS_SUPPORT)
   set_target_properties(ubus_lib PROPERTIES OUTPUT_NAME ubus PREFIX "")
   target_link_options(ubus_lib PRIVATE ${UCODE_MODULE_LINK_OPTIONS})
   target_link_libraries(ubus_lib ${libubus} ${libblobmsg_json})
+  list(APPEND CMAKE_REQUIRED_LIBRARIES ${libubox})
   file(WRITE "${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/test.c" "
     #include <libubus.h>
     int main() { return UBUS_STATUS_NO_MEMORY; }
@@ -166,9 +167,13 @@ if(UBUS_SUPPORT)
   try_compile(HAVE_NEW_UBUS_STATUS_CODES
     ${CMAKE_BINARY_DIR}
     "${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/test.c")
+  check_symbol_exists(uloop_fd_set_cb "libubox/uloop.h" FD_SET_CB_EXISTS)
   if(HAVE_NEW_UBUS_STATUS_CODES)
     add_definitions(-DHAVE_NEW_UBUS_STATUS_CODES)
   endif()
+  if(FD_SET_CB_EXISTS)
+    target_compile_definitions(ubus_lib PUBLIC HAVE_ULOOP_FD_SET_CB)
+  endif()
 endif()

 if(UCI_SUPPORT)
diff --git a/lib/ubus.c b/lib/ubus.c
index 62e7229..36a5674 100644
--- a/lib/ubus.c
+++ b/lib/ubus.c
@@ -665,8 +665,10 @@ uc_ubus_have_uloop(void)
    bool prev = uloop_cancelled;
    bool active;

+#ifdef HAVE_ULOOP_FD_SET_CB
    if (uloop_fd_set_cb)
        return true;
+#endif

    uloop_cancelled = true;
    active = uloop_cancelling();
jow- commented 2 months ago

Merged, thanks!