Crash if receiving messages on streaming call that has finished (1.4.1-async-await.2)

[qusc]

Describe the bug

GRPC server crashes with GRPC/GRPCAsyncServerHandler.swift:529: Fatal error / preconditionFailure().

To reproduce

1. Immediately close client-side streaming call from the server
2. Have the client send more messages

Expected behaviour

Any client behaviour should never cause the server to crash

Additional information

I'm actually not entirely sure in which setup this error occurs. It also doesn't seem to appear to occur immediately (race condition?). I can however reliably reproduce it after a few seconds with my client / server setup. Not closing the streaming call seems to prohibit this error. Also may be noteworthy that the client restarts the streaming call frequently if it finishes since it's supposed to be open all the time.

Full stack trace:

#0	0x00007ff81dbf72d0 in _swift_runtime_on_report ()
#1	0x00007ff81dc762e9 in _swift_stdlib_reportFatalErrorInFile ()
#2	0x00007ff81d936f85 in closure #1 in closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) ()
#3	0x00007ff81d936ca5 in closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) ()
#4	0x00007ff81d935cdc in _assertionFailure(_:_:file:line:flags:) ()
#5	0x00000001000aa3bb in AsyncServerHandler._interceptResponse(_:metadata:) at GRPC/AsyncAwaitSupport/GRPCAsyncServerHandler.swift:529
#6	0x00000001000aa460 in closure #1 in AsyncServerHandler.interceptResponse(_:metadata:) at GRPC/AsyncAwaitSupport/GRPCAsyncServerHandler.swift:539
#7	0x000000010038097c in thunk for @escaping @callee_guaranteed () -> () ()
#8	0x00000001003842b1 in partial apply for thunk for @escaping @callee_guaranteed () -> () ()
#9	0x000000010038099c in thunk for @escaping @callee_guaranteed () -> (@out ()) ()
#10	0x00000001003809e2 in closure #4 in SelectableEventLoop.run() at NIOPosix/SelectableEventLoop.swift:509
#11	0x000000010030713f in thunk for @callee_guaranteed () -> (@error @owned Error) ()
#12	0x0000000100383454 in thunk for @callee_guaranteed () -> (@error @owned Error)partial apply ()
#13	0x000000010037ada2 in closure #1 in withAutoReleasePool<τ_0_0>(_:) at NIOPosix/SelectableEventLoop.swift:25
#14	0x000000010037adec in partial apply for closure #1 in withAutoReleasePool<τ_0_0>(_:) ()
#15	0x00007ff8251cfe6c in autoreleasepool<τ_0_0>(invoking:) ()
#16	0x000000010037ad49 in withAutoReleasePool<τ_0_0>(_:) at NIOPosix/SelectableEventLoop.swift:24
#17	0x000000010037f144 in SelectableEventLoop.run() at NIOPosix/SelectableEventLoop.swift:508
#18	0x000000010034b972 in static MultiThreadedEventLoopGroup.runTheLoop(thread:canEventLoopBeShutdownIndividually:selectorFactory:initializer:_:) at NIOPosix/MultiThreadedEventLoopGroup.swift:87
#19	0x000000010034c022 in closure #1 in static MultiThreadedEventLoopGroup.setupThreadAndEventLoop(name:selectorFactory:initializer:) at NIOPosix/MultiThreadedEventLoopGroup.swift:107
#20	0x00000001003512a3 in partial apply for closure #1 in static MultiThreadedEventLoopGroup.setupThreadAndEventLoop(name:selectorFactory:initializer:) ()
#21	0x000000010034c54f in thunk for @escaping @callee_guaranteed (@guaranteed NIOThread) -> () ()
#22	0x00000001003b43e1 in partial apply for thunk for @escaping @callee_guaranteed (@guaranteed NIOThread) -> () ()
#23	0x00000001003ba4af in closure #1 in static ThreadOpsPosix.run(handle:args:detachThread:) at NIOPosix/ThreadPosix.swift:105
#24	0x00000001003ba5a9 in @objc closure #1 in static ThreadOpsPosix.run(handle:args:detachThread:) ()
#25	0x0000000102fb0cd0 in _pthread_start ()
#26	0x0000000102fb8cff in thread_start ()

[glbrntt]

We're hitting a precondition failure when a response is being written by the user provided function.

It's a precondition because we believe that in the .completed state we should have, well, completed the RPC. That's not quite right. We usually enter this state for two reasons:

• We're done writing responses and can now send the final status, and
• If there's a connection error or some other issue that means we need to cancel the user function before it finishes writing.

We shouldn't precondition failure for that second case (as we currently do) as the user function runs in a Task and is cancelled asynchronously (we don't await cancellation). This means it's possible for the user function to attempt to send a response after we've changed state. I think we can just lower the precondition failure to a no-op.

[glbrntt]

This doesn't really tie in with your description though...

Immediately close client-side streaming call from the server

Do you mean have the server just immediately return in the client-streaming call handler? Or something else?

[qusc]

Yes, that's what I meant.
Also thanks for your explanation

[glbrntt]

This should be fixed in https://github.com/grpc/grpc-swift/releases/tag/1.4.1-async-await.3 -- please reopen if that's not the case!