In the test for client Call failing with ErrClosed on a closed
server, wait for the client's OnClose handler to get triggered
to make sure closing the socket had properly been administered
on the client's side. Otherwise trying a new Call() might fail
with some other error than ErrClosed, for instance ENOTCONN.
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Simplify close idle connections logic in server shutdown to be more
intuitive. Modify add connection logic to check if server has been
shutdown before adding any new connections. Modify test to make all
calls before server shutdown.
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
Make `TestServerRequestTimeout` deadline assertion log print expected
and actual values in the same format for more clear troubleshooting.
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
Implementation of the 1.2 protocol with support for streaming. Provides
the client and server interfaces for implementing services with
streaming.
Unary behavior is mostly unchanged and avoids extra stream tracking just
for unary calls. Streaming calls are tracked to route data to the
appropriate stream as it is received.
Stricter stream ID handling, disallowing unexpected re-use of stream
IDs.
Signed-off-by: Derek McGowan <derek@mcg.dev>
This test occasionally fails on Windows. This commit logs extra
information about the error to understand the situation better.
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
This change replaces github.com/gogo/protobuf with
google.golang.org/protobuf, except for the code generators.
All proto-encoded structs are now generated from .proto files,
which include ttrpc.Request and ttrpc.Response.
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
gRPC has UNIMPLEMENTED status code that indicates a requested method
is not implemented. However ttrpc was returning codes.NotFound which
could be returned when a request entity was not there.
This fix changes the status code from codes.NotFound to
codes.Unimplemented to disambiguate and be more compatible with gRPC.
Fixes#80.
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
ttrpc provides WithOnClose option for user and ttrpc will call the
callback function when connection is closed unexpectedly or the ttrpc
client's Close() method is called. containerd runtime plugin uses it
to handle cleanup the resources created by containerd shim.
But the ttrpc client's Close() is only trigger and the shim's cleanup
resource callback is called asynchronously, which might make part of
resources leaky. There is an example from containerd-runtime-v2 for
runc:
```happy
[Task.Delete goroutine] [cleanupCallback goroutine]
call ttrpc client.Close() -->
read bundle and call runc delete
delete bundle
```
If the cleanupCallback is called after deleting bundle, the callback
will fail to call runc delete. If there is any running processes, the
resource becomes leaky.
```unhappy
[Task.Delete goroutine] [cleanupCallback goroutine]
call ttrpc client.Close() -->
delete bundle
failed to read bundle and call runc delete
```
In order to avoid this, introduces the UserOnCloseWait to make sure that
the cleanupCallback has been called synchronously, like:
```
[Task.Delete goroutine] [cleanupCallback goroutine]
call ttrpc client.Close() -->
wait for callback
read bundle and call runc delete
<-- finish sync
delete bundle
```
Signed-off-by: Wei Fu <fuweid89@gmail.com>
This copies the codes and status package from grpc as it is the only references
to the grpc project from ttrpc. This will help ensure that API breaking changes
in grpc do not affect ttrpc.
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Currently the EOF that the server gets when a client connection is
closed is being ignored, which means that the goroutine that is
processing the client connection will never exit. If it never exits
it will never close the underlying unix socket and this would lead
to a file descriptor leak.
Signed-off-by: Georgi Sabev <georgethebeatle@gmail.com>
Adds a new field to the `Request` type which specifies a timeout (in
nanoseconds) for the request. This is propagated on method dispatch as a
context timeout.
There was some discussion here on supporting a broader "metadata" field
(similar to grpc) that can be used for other things, but we ended up
with a dedicated field because it is lighter weight and expect it to be
used pretty heavily as is.... metadata may be added in the future, but
is not necessary for timeouts.
Also discussed using a deadline vs a timeout in the request and decided
to go with a timeout in order to deal with potential clock skew between
the client and server. This also has the side-effect of eliminating the
protocol/wire overhead from the request timeout.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
To gracefully handle scenarios where the connection is closed or the
client is closed, we now set the final error to be `ErrClosed`. Callers
can resolve it through using `errors.Cause` to detect this condition.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
Because of issues with glibc, using the `os/user` package can cause when
calling `user.Current()`. Neither the Go maintainers or glibc developers
could be bothered to fix it, so we have to work around it by calling the
uid and gid functions directly. This is probably better because we don't
actually use much of the data provided in the `user.User` struct.
This required some refactoring to have better control over when the uid
and gid are resolved. Rather than checking the current user on every
connection, we now resolve it once at initialization. To test that this
provided an improvement in performance, a benchmark was added.
Unfortunately, this exposed a regression in the performance of unix
sockets in Go when `(*UnixConn).File` is called. The underlying culprit
of this performance regression is still at large.
The following open issues describe the underlying problem in more
detail:
https://github.com/golang/go/issues/13470https://sourceware.org/bugzilla/show_bug.cgi?id=19341
In better news, I now have an entire herd of shaved yaks.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
Because ttrpc can be used with abstract sockets, it is critical to
ensure that only certain users can connect to the unix socket. This is
of particular interest in the primary use case of containerd, where a
shim may run as root and any user can connection.
With this, we get a few nice features. The first is the concept of a
`Handshaker` that allows one to intercept each connection and replace it
with one of their own. The enables credential checks and other measures,
such as tls. The second is that servers now support configuration. This
allows one to inject a handshaker for each connection. Other options
will be added in the future.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
The request and response requests opened up a nasty race condition where
waiters could find themselves either blocked or receiving errant errors.
The result was low performance and inadvertent busy waits. This
refactors the client to have a single request into the main client loop,
eliminating the race.
The reason for the original design was to allow a sender to control
request and response individually to make unit testing easier. The unit
test has now been refactored to use a channel to ensure that requests
are serviced on graceful shutdown.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
This change increases the maximum message size to 4MB to be inline
with the grpc default. The buffer management approach has been changed
to use a pool to minimize allocations and keep memory usage low.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
This apples logic to correctly Close a server, as well as implements
graceful shutdown. This ensures that inflight requests are not
interrupted and works similar to the functionality in `net/http`.
This required a fair bit of refactoring around how the connection is
managed. The connection now has an explicit wrapper object, ensuring
that shutdown happens in a coordinated fashion, whether or not a
forceful close or graceful shutdown is called.
In addition to the above, hardening around the accept loop has been
added. We now correctly exit on non-temporary errors and debounce the
accept call when encountering repeated errors. This should address some
issues where `SIGTERM` was not honored when dropping into the accept
spin.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
Improve the test to conduct two concurrent calls. The test server now
just doubles the input and we use a unix socket to better represent what
will be used in production.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
Rather than employ the typeurl package, we now generate code to
correctly allocate the incoming types from the caller. As a side-effect
of this activity, the services definitions have been split out into a
separate type that handles the full resolution and dispatch of the
method, incuding correctly mapping the RPC status.
This work is a pre-cursor to larger protocol change that will allow us
to handle multiple, concurrent requests.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
The server unit test now manually mocks much of the generated code for a
given service, including back registration of the service. We avoid
having a package-level global descriptor in favor of a closures for the
handler dispatch function.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
With this change, we define a simple server and client framework to
start generating code against. We define a simple handler system with
back registration into the server definition.
From here, we can start generating code against the handlers.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
