grpc-proxy: Re-work bound_ok/bound_fail events handling 67/29567/4
authorMarius Vlad <marius.vlad@collabora.com>
Fri, 29 Dec 2023 20:16:37 +0000 (22:16 +0200)
committerMarius Vlad <marius.vlad@collabora.com>
Tue, 2 Jan 2024 17:29:18 +0000 (19:29 +0200)
commitae3ef78cb1a01b690917eb8d93a3e68517a7a93d
treee16452d6fc2dffd2e169d7d77fd0cb1dec1b4185
parent071440ef54444c3be2aa72a402563a97cc350e3b
grpc-proxy: Re-work bound_ok/bound_fail events handling

This bigger patch changes the way gRPC proxy client connects to the
server, more specifically how it binds to the same agl_shell interface
as the shell client.

I debated if this should be split into more pieces but I think it makes
more sense to have contained into a single patch as it doesn't make
sense to have some bits off and some bits on, as both the server and the
client side need to be changed at the same time.

This biggest change with this patch is to avoid a potential race condition
between the gRPC proxy client and the shell-client, but also to simplify
the way the gRPC client connects to the server as there aren't sufficient
guards in the server to the provent various corner cases to take place.
Rather than attempting to fix that, simplifying the entire bit and
allow only a single agl-shell client and a sigle gRPC proxy client
connected at a single time and remove any other potential clients
trying to connect at all, later on.

Context and usage:

Both the gRPC proxy and shell-client bind to the same agl_shell
interface, meaning that both use the shell-client (homescreen,
flutter-ics-homescreen, flutter-auto, wam) and the gRPC proxy can
issue agl_shell requests, with some minor differences --
setting background and panels which do not make sense for
gRPC proxy client.

The compositor can signal back to clients when a bind was OK or
not with the bound_ok/bound_fail event. Previously to
adding the bound_ok/bound_fail events, any other client trying to use
agl_shell interface, more than once, would get a protocol violation.
With the bound_ok/bound_fail events clients would instead receive this
event and could theoretically act upon it.

The race that this patch tries to overcome is that the gRPC client will
periodically attempt to verify if there's a shell-client already
connected to server by binding to agl_shell interface and waiting for
bound_fail event. In case it got bound_ok, it would disconnect and
attempt at latter point in time, until it got the bound_failed event, and thus
signalling that there's shell client already connected, allowing to
proceed further.

This worked fine most of the time, depending on how fast the shell
client also bind to agl_shell.  For a brief period of time, it might be
that shell client also gets a bound_fail, requiring it to retry at a
later point in time. The disconnected/reconnect of the gRPC proxy would
then complicate matters as the client resources will get overwritten. So
rather than trying to try that fix that, a better approach is to
simplify the way this works entirely.

The change in this patch is that the gRPC proxy doesn't
connect/disconnect periodically but rather it waits for the compositor
to signal when it is ready to bind to agl_shell interface.

That happens with the help of the doas event status, from the
agl_shell_ext interface. If this event status is alright (OK) then it means
the gRPC can then bind to agl_shell interface.

If it gets a failed status it would just wait and then issue another
doas request and continue as such forever until it got an alright
status. The distinct is that in this case the gRPC proxy would not disconnect
and then reconnect retrying the same thing.

This change to simplify some of the assumption in server, and
client side implementation would just race with the shell client when
binding to the agl_shell interface.

Bug-AGL: SPEC-4977
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Change-Id: Ib74f789553d3b130ee8e61d0068e617dc2209a58
grpc-proxy/log.h
grpc-proxy/main-grpc.cpp
grpc-proxy/main-grpc.h
src/ivi-compositor.h
src/shell.c