[OpenWrt-Devel] ubus: potential regression in request processing

[Zefir Kurtisi]

Hello all,

After upgrading from a rather old commit (4c4f35cf2230) to HEAD, I observe a
change in the ubus processing of requests / notifications which to me seems at
least strange if not wrong.

The scenario used is simple:
1) server with request handler that
   a) first sends a notification
   b) followed by response
2) client
   a) issuing a synchronous request via ubus_invoke()
   b) listening to notifications

The previous behaviour was that notifications sent by server during a request were
ignored (i.e. not reaching the client's notification handler). With the latest
version, the client receives the notification in the middle of the request, i.e.
from within ubus_invoke().

This is problematic in particular if the clients issues follow-up requests from
within its notification-handler - which generally should be ok if one expects
synchronous processing.


I don't understand ubus internals well enough to figure out if this is a bug or a
feature. Bisecting points to:

> commit 2d660c519d2fcff95248da9f4fd9b37d61f9eb09
> Author: Felix Fietkau <nbd at openwrt.org>
> Date:   Thu Jan 22 13:30:20 2015 +0100
> 
>     libubus: fix processing queued messages after blocking requests
>     
>     Signed-off-by: Felix Fietkau <nbd at openwrt.org>
> 
> diff --git a/libubus-req.c b/libubus-req.c
> index d44db51..2197f2a 100644
> --- a/libubus-req.c
> +++ b/libubus-req.c
> @@ -175,7 +175,7 @@ int ubus_complete_request(struct ubus_context *ctx, struct ubus_request *req,
>         if (!registered) {
>                 uloop_fd_delete(&ctx->sock);
>  
> -               if (ctx->stack_depth)
> +               if (!ctx->stack_depth)
>                         ctx->pending_timer.cb(&ctx->pending_timer);
>         }
>  
> 

which ensures that an initially unregistered client receives pending notifications
from within ubus_invoke() - which at one hand seems required (the client is
unregistered again and won't get the notification otherwise), but on the other
hand breaks the synchronism strictness.

Wouldn't it be better to drop processing of pending notifications completely at
that point?


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