Question: Is it okay to call finish with an outstanding read in the server?

[lklein53]

Hi,

I want to clarify if its okay to call finish with an outstanding read in the server.
From the best practices section item 24 of grpc-cpp i think it should be okay.

Is it legal for the server to call Finish while there is a Read or Write outstanding?
This is fine for reads and OnReadDone(ok=false) is called. Calling Finish with a write outstanding is not valid API usage, since a Finish can be considered a final write with no data, which would violate the one-write-in-flight rule.

The documentation of asio grpc however states

• May not be called currently with finish/write_and_finish. It is not meaningful to call it concurrently with
  another read on the same rpc since reads on the same stream are delivered in order.

Is running finish and read on a strand okay?

What i want to achieve is a bidirectional grpc server that finishes a rpc with a status code (not cancellation) even if the client has not sent all requests or hasn't called writesDone.

thanks and best regards,
Lars

[Tradias]

Since asio-grpc's documentation is just a copy of the official gRPC documentation, we can look there for answers. Here is bidi-streaming Read function (highlight by me):

Read a message of type R into msg.
Completion will be notified by tag on the associated completion queue. This is thread-safe with respect to Write or WritesDone methods. It should not be called concurrently with other streaming APIs on the same stream. It is not meaningful to call it concurrently with another AsyncReaderInterface::Read on the same stream since reads on the same stream are delivered in order.
Parameters
[out] msg Where to eventually store the read message.
[in] tag The tag identifying the operation.
Side effect: note that this method attempt to receive initial metadata for a stream if it hasn't yet been received.

https://grpc.github.io/grpc/cpp/classgrpc_1_1_server_async_reader_writer_interface.html#ae63d347d413c401b1e976f165efde50b

We can also take a look at Finish but it is not very helpful: https://grpc.github.io/grpc/cpp/classgrpc_1_1_server_async_reader_writer_interface.html#ae63d347d413c401b1e976f165efde50b

I think the gRPC documentation is not very clear, what is meant by other streaming APIs? I assumed they meant every other function, e.g. ReadInitialMetadata, Finish and so on, that's why I wrote it the way I did in the asio-grpc documentation.
I added a test to see what really happens when you call read and finish concurrently: 420bf11

[Tradias]

Looks like calling read and finish concurrently does not cause a crash. Note that my tests are compiled in Release, so no assertions. I recommend that you test it yourself in Debug with assertions. Let me know the outcome then I can update asio-grpc documentation.

[lklein53]

I ran the tests in a debug build and they passed just fine.

[Tradias]

Looks like I have misunderstood your question. It is in fact OK to initiate a finish after initiating a read but it is not OK to initiate finish concurrently with initiating read (that's also what the gRPC documentation says). Let's look at an example:

rpc.read(request, [](bool) {});
// OK, read is initiated and may be outstanding, we can initiate finish:
co_await rpc.finish(...);
// Looks like gRPC even guarantees that `read()` will have completed before we get here

Compare that to:

std::thread t{[&] {
  rpc.read(request, [](bool) {});  // Not OK. In fact this could end up initiating read() after finish() completed which is disallowed anyways
}};
co_await rpc.finish(...);

[lklein53]

okay thanks for clarifying. I will close the question