Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ubus: fix uc_ubus_have_uloop for eloop+uloop combination #198

Merged
merged 1 commit into from
May 2, 2024

Conversation

nbd168
Copy link
Contributor

@nbd168 nbd168 commented May 1, 2024

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
Copy link
Contributor Author

nbd168 commented May 1, 2024

The failed CI run looks like outdated libubox to me

@jow-
Copy link
Owner

jow- commented May 2, 2024

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();

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

Signed-off-by: Felix Fietkau <[email protected]>
@jow- jow- merged commit 25d41ba into jow-:master May 2, 2024
6 checks passed
@jow-
Copy link
Owner

jow- commented May 2, 2024

Merged, thanks!

@nbd168 nbd168 deleted the ubus-uloop-fix branch May 2, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants