[OpenWrt-Devel] Mismatch in ubus / uloop options

[Felix Fietkau]

On 2015-12-07 10:41, Karl Vogel wrote:
> In ubus_reconnect, the ctx->sock.fd is created and set to non-blocking mode:
> 
>     fcntl(ctx->sock.fd, F_SETFL, fcntl(ctx->sock.fd, F_GETFL) |
>     O_NONBLOCK | O_CLOEXEC);
> 
> 
> http://git.openwrt.org/?p=project/ubus.git;a=blob;f=libubus-io.c;h=9d3ac6c33f4d926c7f5c5d2033d531fc5fc6f834;hb=259450f414d8c9ee41896e8e6d6bc57ec00e2b63#l396
> 
> 
> However, further in the call stack, the socket will get added to uloop
> via ubus_add_uloop with the flag ULOOP_BLOCKING set.
> 
> http://git.openwrt.org/?p=project/ubus.git;a=blob;f=libubus.h;h=54e7d481b0496e719f2f567b664a2619c82a3b16;hb=259450f414d8c9ee41896e8e6d6bc57ec00e2b63#l245
> 
> ie when called from ubus_complete_request on an !registered ctx->sock,
> the ubus_add_uloop is done:
> 
> http://git.openwrt.org/?p=project/ubus.git;a=blob;f=libubus-req.c;h=bf19f3670ea5b215190dfcce125c358f616c72a4;hb=259450f414d8c9ee41896e8e6d6bc57ec00e2b63#l136
> 
> The uloop function assumes a socket is blocking by default and will
> never make a socket blocking again, it will only turn it to non-blocking
> if ULOOP_BLOCKING isn't set. This caused a busy loop in some daemons
> that had their socket set to non-blocking due to another issue, ie. poll
> using 0 as infinite instead of -1, for which I submitted a fix earlier.
> 
> I wonder if it wouldn't be better to have uloop set the socket flags in
> all cases? Currently it "assumes" the socket is blocking and if
> ULOOP_BLOCKING is passed, it will set it to non-blocking, but it won't
> do the other way round. Though that might have some impact on daemons
> that think the socket is non-blocking while it will be turned into
> blocking if this is changed. So not sure what the best way to go about
> this one is?!
I think this uloop adding/removing of sockets in ubus is just a leftover
that should be removed. Needs some testing though.

These calls were added in combination with code that ran uloop_run()
inside blocking requests. I got rid of that due to recursion issues and
switched to using poll() directly, but apparently forgot to clean up the
adding/removing of sockets.

- Felix
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel