Some gRPC calls can crash the server #4197
Comments
Closed
Kissaki
added a commit
to mumble-voip/mumble-www
that referenced
this issue
Jun 8, 2020
As it is not included in default and official builds, and it still has some unfixed issues. https://github.com/mumble-voip/mumble/issues?q=is%3Aopen+is%3Aissue+label%3AgRPC mumble-voip/mumble#4197
Krzmbrzl
added a commit
to Krzmbrzl/mumble
that referenced
this issue
Feb 6, 2022
The gRPC implementation never left the experimental state and never reached a properly stable state to the point where we would feel good about enabling it by default. In addition to that, there has been no further attempts at finding and fixing the encountered issues in the implementation (except mumble-voip#3947 but that was discontinued). As such we had an essentially unmaintained piece of code in our server implementation that was known to be buggy and that nobody wanted to fix. In addition to that the implementation itself could not be considered very clean or elegant and therefore only represented a few smelly corners in our code base. For this reason, we decided to remove the gRPC support entirely from Mumble (for now). What we hope to gain by that is: - Prevent people from building unstable server versions and then coming to us complaining that it crashed/misbehaved - Removing (essentially) dead code - Reduce the RPC implementation complexity That last piece is crucial: By removing gRPC support we reduce the amount of supported RPC frameworks to only one (ignoring DBus for now). Our future plans include a refactoring of how RPC is being handled and implemented and only having to worry about maintaining compatibility with one RPC system is much easier than having to worry about two (with (slightly) different APIs). Once the RPC implementation has been rewritten, more RPC backends may be reintroduced and in that process we might investigate adding a proper gRPC implementation to the code (that then hopefully is more stable than the current one). Fixes mumble-voip#4567 Fixes mumble-voip#4197 Fixes mumble-voip#3496 Fixes mumble-voip#3429 Fixes mumble-voip#3265
1 task
Krzmbrzl
added a commit
to Krzmbrzl/mumble
that referenced
this issue
Mar 16, 2022
The gRPC implementation never left the experimental state and never reached a properly stable state to the point where we would feel good about enabling it by default. In addition to that, there has been no further attempts at finding and fixing the encountered issues in the implementation (except mumble-voip#3947 but that was discontinued). As such we had an essentially unmaintained piece of code in our server implementation that was known to be buggy and that nobody wanted to fix. In addition to that the implementation itself could not be considered very clean or elegant and therefore only represented a few smelly corners in our code base. For this reason, we decided to remove the gRPC support entirely from Mumble (for now). What we hope to gain by that is: - Prevent people from building unstable server versions and then coming to us complaining that it crashed/misbehaved - Removing (essentially) dead code - Reduce the RPC implementation complexity That last piece is crucial: By removing gRPC support we reduce the amount of supported RPC frameworks to only one (ignoring DBus for now). Our future plans include a refactoring of how RPC is being handled and implemented and only having to worry about maintaining compatibility with one RPC system is much easier than having to worry about two (with (slightly) different APIs). Once the RPC implementation has been rewritten, more RPC backends may be reintroduced and in that process we might investigate adding a proper gRPC implementation to the code (that then hopefully is more stable than the current one). Fixes mumble-voip#4567 Fixes mumble-voip#4197 Fixes mumble-voip#3496 Fixes mumble-voip#3429 Fixes mumble-voip#3265
Krzmbrzl
added a commit
that referenced
this issue
Mar 16, 2022
The gRPC implementation never left the experimental state and never reached a properly stable state to the point where we would feel good about enabling it by default. In addition to that, there has been no further attempts at finding and fixing the encountered issues in the implementation (except #3947 but that was discontinued). As such we had an essentially unmaintained piece of code in our server implementation that was known to be buggy and that nobody wanted to fix. In addition to that the implementation itself could not be considered very clean or elegant and therefore only represented a few smelly corners in our code base. For this reason, we decided to remove the gRPC support entirely from Mumble (for now). What we hope to gain by that is: Prevent people from building unstable server versions and then coming to us complaining that it crashed/misbehaved Removing (essentially) dead code Reduce the RPC implementation complexity That last piece is crucial: By removing gRPC support we reduce the amount of supported RPC frameworks to only one (ignoring DBus for now). Our future plans include a refactoring of how RPC is being handled and implemented and only having to worry about maintaining compatibility with one RPC system is much easier than having to worry about two (with (slightly) different APIs). Once the RPC implementation has been rewritten, more RPC backends may be reintroduced and in that process we might investigate adding a proper gRPC implementation to the code (that then hopefully is more stable than the current one). Fixes #4567 Fixes #4197 Fixes #3496 Fixes #3429 Fixes #3265
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
PR #3947 mentions that some gRPC calls may or will crash the server.
It is unclear/undocumented which ones do, and under what conditions.
Note for concerned users: gRPC is not currently enabled by default, meaning not distributed enabled in our binary releases.
The text was updated successfully, but these errors were encountered: