Reusing ClientContext for multiple calls crashes LabVIEW

[bkeryan]

Reusing a ClientContext for multiple unary calls crashes LabVIEW.

Top of call stack:

	labview_grpc_server.dll!abort() Line 77	C++
>	labview_grpc_server.dll!grpc::ClientContext::set_call(grpc_call * call, const std::shared_ptr<grpc::Channel> & channel) Line 117	C++
	labview_grpc_server.dll!grpc::Channel::CreateCallInternal(const grpc::internal::RpcMethod & method, grpc::ClientContext * context, grpc::CompletionQueue * cq, unsigned __int64 interceptor_pos) Line 152	C++
	labview_grpc_server.dll!grpc::Channel::CreateCall(const grpc::internal::RpcMethod & method, grpc::ClientContext * context, grpc::CompletionQueue * cq) Line 160	C++
	labview_grpc_server.dll!grpc::internal::BlockingUnaryCallImpl<grpc_labview::LVMessage,grpc_labview::LVMessage>::BlockingUnaryCallImpl<grpc_labview::LVMessage,grpc_labview::LVMessage>(grpc::ChannelInterface * channel, const grpc::internal::RpcMethod & method, grpc::ClientContext * context, const grpc_labview::LVMessage & request, grpc_labview::LVMessage * result) Line 70	C++
	labview_grpc_server.dll!grpc::internal::BlockingUnaryCall<grpc_labview::LVMessage,grpc_labview::LVMessage,grpc_labview::LVMessage,grpc_labview::LVMessage>(grpc::ChannelInterface * channel, const grpc::internal::RpcMethod & method, grpc::ClientContext * context, const grpc_labview::LVMessage & request, grpc_labview::LVMessage * result) Line 52	C++

This assertion is in the gRPC C Core, which documents that ClientContext instances should not be reused across RPCs.

I think it would be helpful for ClientUnaryCall, ClientBeginClientStreamingCall, etc. to check whether clientContext->gRPCClientContext.call() is null and return an error if it's not. That way, G developers don't have to rebuild labview_grpc_server.dll to figure out what's wrong with their code.

AB#2272018

[AndrewHeim]

As of 1.0.1.1, this appears to have been fixed. Best I can tell, anyways.

Reusing it (on my machine, with local loopback) gets me a round trip time of 142 us. Opening a new one gets me a round trip time averaging 560 us. So it appears I'm shortcutting something by reusing it.

If reusing it is doing something wrong, I don't see any sign of a problem, other than going faster.

[bkeryan]

The gRPC C Core's policy on reusing client contexts hasn't changed and is unlikely to change. The assertion is still present: https://github.com/grpc/grpc/blob/master/src/cpp/client/client_context.cc#L128

grpc_client.cc doesn't seem to have any code for resetting/reallocating the gRPC client context. Are assertions still enabled?

[AndrewHeim]

I can't speak to whether assertions are enabled. But I can say that #320 and #193 acted up more often when I reused the grpc context.

That doesn't seem to make immediate sense to me, as the race condition behind those issues looks like it would be unaffected by reusing the context - it's about how quickly the call to the server returns. Or maybe it slowed things down enough that I ran fewer iterations in a few minutes of testing and reduced the likelihood I'd see the issue in a given amount of testing time? I can't say.