containerd / ttrpc-rust

Rust implementation of ttrpc (GRPC for low-memory environments)
Apache License 2.0
195 stars 45 forks source link

Need a mechanism which can help method.handler to realize the client has been closed #180

Closed yuqitao closed 1 year ago

yuqitao commented 1 year ago

Which feature do you think can be improved?

We need a mechanism which can help method.handler to realize the client has been closed in sync ttrpc scenario, like context.Done in golang ttrpc.

Current implementation in sync ttrpc server:

listener_loop thread
-- accept
-- client_handler thread
-- -- response thread
-- -- method_handler thread: default 3
-- -- block wait for method handler and response thread exit

In containerd task wait scenario, like https://github.com/containerd/containerd/blob/f7f2be732159a411eae46b78bfdb479b133a823b/pkg/process/init.go#L212

The method_handler will hang until the container exit. When restart containerd, then the ttrpc client closes, new ttrpc client will create new client_handler thread. But the old client_handler thread , response thread and task wait method_handler thread will leak.

How can it be improved?

method_handler threads read message from connection fd and handle request individually. Both recv(fd) and handle request logic are one thread,the handle request logic hang, the thread will never reach recv(fd), this thread will never get socket disconnected error.

Maybe we can add a channel rx in TtrpcContext, and the user can realize client disconnected by get RecvError from rx.recv.

Another different way, can we find a way to check fd in TtrpcContext to get socket status ? @Tim-Zhang @fuweid

Tim-Zhang commented 1 year ago

Good catch @yuqitao