Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Server should send RST_STREAM when deadline is exceeded #2886

Open
ejona86 opened this issue Jun 27, 2019 · 4 comments · May be fixed by #8071
Open

Server should send RST_STREAM when deadline is exceeded #2886

ejona86 opened this issue Jun 27, 2019 · 4 comments · May be fixed by #8071
Assignees
@arjan-bal
Labels
Area: Transport Includes HTTP/2 client/server and HTTP server handler transports and advanced transport features. fixit P2 Type: Bug

Comments

@ejona86
Copy link
Member

ejona86 commented Jun 27, 2019

The server code relies on the application to return when the deadline is exceeded. But when it does return, it doesn't cancel the RPC; it sends a graceful close:
https://github.com/grpc/grpc-go/blob/v1.21.1/server.go#L1010

If the deadline is exceeded when the handler returns, the RPC should be cancelled and RST_STREAM should be sent (if one hasn't already been received from the client).
@ejona86
Copy link
Member Author

ejona86 commented Jun 27, 2019

Note that this may have interactions with ServerInHandle, if ServerInHandle changes the deadline. It is only appropriate to use RST_STREAM when the client specified its deadline, since in that case the client will ignore what the server sends anyway. If the ServerInHandler reduced the deadline, then a normal close is appropriate.

@stale stale bot added the stale label Sep 6, 2019
@dfawley dfawley removed the stale label Sep 6, 2019
@grpc grpc deleted a comment from stale bot May 3, 2021
@menghanl menghanl removed the fixit label May 3, 2021
@infovivek2020
Copy link
Contributor

@purnesh42H Please assign to me

@eshitachandwani eshitachandwani added the Area: Server Includes Server, Streams and Server Options. label Sep 4, 2024
@misvivek misvivek mentioned this issue Dec 3, 2024
Closed
@janardhanvissa
Copy link
Contributor

@purnesh42H please assign me this issue

@arjan-bal arjan-bal assigned arjan-bal and unassigned infovivek2020 Jan 21, 2025
@arjan-bal
Copy link
Contributor

I was going through the code to understand the parts responsible for sending RST_STREAM frames on context timeouts. It seems that only gRPC Go clients send RST_STREAMs on context expiration though two different paths.

First path

To read data from a stream, both server and client streams call the read method on the shared Stream struct.

grpc-go/internal/transport/client_stream.go

Lines 54 to 60 in ee3e8d9

func (s *ClientStream) Read(n int) (mem.BufferSlice, error) {
b, err := s.Stream.read(n)
if err == nil {
s.ct.incrMsgRecv()
}
return b, err
}

grpc-go/internal/transport/server_stream.go

Lines 53 to 59 in ee3e8d9

func (s *ServerStream) Read(n int) (mem.BufferSlice, error) {
b, err := s.Stream.read(n)
if err == nil {
s.st.incrMsgRecv()
}
return b, err
}

Stream.read() eventually calls the recvBufferReader.Read. This method has different paths for server and client streams:

grpc-go/internal/transport/transport.go

Lines 167 to 171 in ee3e8d9

if r.closeStream != nil {
buf, r.err = r.readClient(n)
} else {
buf, r.err = r.read(n)
}

readClient calls r.closeStream() if it detects a context timeout.

grpc-go/internal/transport/transport.go

Lines 220 to 241 in ee3e8d9

func (r *recvBufferReader) readClient(n int) (buf mem.Buffer, err error) {
// If the context is canceled, then closes the stream with nil metadata.
// closeStream writes its error parameter to r.recv as a recvMsg.
// r.readAdditional acts on that message and returns the necessary error.
select {
case <-r.ctxDone:
// Note that this adds the ctx error to the end of recv buffer, and
// reads from the head. This will delay the error until recv buffer is
// empty, thus will delay ctx cancellation in Recv().
//
// It's done this way to fix a race between ctx cancel and trailer. The
// race was, stream.Recv() may return ctx error if ctxDone wins the
// race, but stream.Trailer() may return a non-nil md because the stream
// was not marked as done when trailer is received. This closeStream
// call will mark stream as done, thus fix the race.
//
// TODO: delaying ctx error seems like a unnecessary side effect. What
// we really want is to mark the stream as done, and return ctx error
// faster.
r.closeStream(ContextErr(r.ctx.Err()))
m := <-r.recv.get()
return r.readAdditional(m, n)

r.closeStream is set only for grpc clients and calls the ClientStream.Close method.

grpc-go/internal/transport/client_stream.go

Lines 62 to 73 in ee3e8d9

// Close closes the stream and popagates err to any readers.
func (s *ClientStream) Close(err error) {
var (
rst bool
rstCode http2.ErrCode
)
if err != nil {
rst = true
rstCode = http2.ErrCodeCancel
}
s.ct.closeStream(s, err, rst, rstCode, status.Convert(err), nil, false)
}

Second path

For non-unary streams on the client side, a separate goroutine is spawned which monitors the channel and stream context cancellations and call cs.finish() which eventually calls the ClientStream.Close method.

grpc-go/stream.go

Lines 388 to 402 in ee3e8d9

if desc != unaryStreamDesc {
// Listen on cc and stream contexts to cleanup when the user closes the
// ClientConn or cancels the stream context. In all other cases, an error
// should already be injected into the recv buffer by the transport, which
// the client will eventually receive, and then we will cancel the stream's
// context in clientStream.finish.
go func() {
select {
case <-cc.ctx.Done():
cs.finish(ErrClientConnClosing)
case <-ctx.Done():
cs.finish(toRPCErr(ctx.Err()))
}
}()
}

@dfawley dfawley added Area: Transport Includes HTTP/2 client/server and HTTP server handler transports and advanced transport features. and removed Area: Server Includes Server, Streams and Server Options. labels Feb 4, 2025
@arjan-bal arjan-bal linked a pull request Feb 10, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@arjan-bal arjan-bal

Labels
Area: Transport Includes HTTP/2 client/server and HTTP server handler transports and advanced transport features. fixit P2 Type: Bug
Projects
None yet
Milestone
No milestone
8 participants
@menghanl @ejona86 @arvindbr8 @infovivek2020 @dfawley @arjan-bal @janardhanvissa @eshitachandwani