Using AsyncNotifyWhenDone and IsCancelled with async server

[Przemysław Sobala]

Hello

I wanted to make use of IsCancelled in async server to stop processing requests that were cancelled by user.

In order to use ServerContext::IsCancelled method I had to call ServerContext::AsyncNotifyWhenDone first.

And then some problems occurred.

1. I took a greeter_async_server.cc from grpc cpp examples and added ctx.AsyncNotifyWhenDone(this);

https://github.com/pszemus/grpc/blob/79b1464b277d6e438fd9032addc5d7224cc9f56b/examples/cpp/helloworld/greeter_async_server.cc#L97

After executing a request the server crashes (https://github.com/grpc/grpc/issues/20155).

If I drop deleting the CallData object in FINISH state it turns out that there are 2 events from CompletionQueue at the end of request processing:

Server listening on 0.0.0.0:50051

Proceeding call 0x4e25550 with status 0
Proceeding call 0x4e25550 with status 1
Proceeding call 0x4e25550 with status 2
Proceeding call 0x4e25550 with status 2

So I think that's why the server crashes - with the first FINISH state the CallData is being freed and the second event cannot operate on deleted object.

2. If there're always 2 events from CompletionQueue at the end I can introduce new transitive state of CallData.

https://github.com/pszemus/grpc/blob/f5db1153b78626da4460a6cdaa2f11e48a78dcab/examples/cpp/helloworld/greeter_async_server.cc#L116

Then the request is processed through all 4 states (and CallData is freed only once at the last one of them):

Proceeding call 0x4e25550 with status 0
Proceeding call 0x4e25550 with status 1
Proceeding call 0x4e25550 with status 2
Proceeding call 0x4e25550 with status 3

Almost OK...

3. As said in https://github.com/pszemus/grpc/blob/f5db1153b78626da4460a6cdaa2f11e48a78dcab/examples/cpp/helloworld/greeter_async_server.cc#L149 I want to span HandleRpcs across multiple threads.

Without AsyncNotifyWhenDone it works really great, but after enabling it, those 2 events sent at the end of request processing, can be received by 2 seperate threads, leading to race condition while changing the state of CallData and not being freed at all (CallData maintains in FINISH state and never proceeds to CLEANUP state).

https://github.com/pszemus/grpc/blob/b7839549fead5090958eb3539f8754d69eb532e8/examples/cpp/helloworld/greeter_async_server.cc#L70

[140135346489088] Proceeding call 0x7f73b800a090 with status 0
[140135346489088] Proceeding call 0x7f73a8000b20 with status 1
[140135346489088] Proceeding call 0x7f73a8000b20 with status 2
[140134984972032] Proceeding call 0x7f73a8000b20 with status 3
^^^^^^^^^^^^^^^^^
different thread

Worst case scenario looks like that:

[140135346489088] Proceeding call 0x7f73b800a090 with status 0
[140135346489088] Proceeding call 0x7f73a8000b20 with status 1
[140135346489088] Proceeding call 0x7f73a8000b20 with status 2
[140134984972032] Proceeding call 0x7f73a8000b20 with status 2

2 different threads modify CallData's status at the same time resulting not being freed at all.

What can be done to safely free CallData while getting additional events from CompletionQueue after enabling AsyncNotifyWhenDone?

--

Best regards

Przemysław Sobala

[Yash Tibrewal]

I would suggest passing a different value in the tag parameter of AsyncNotifyWhenDone. That way you would know whether the tag returned is for one of the existing states of the RPC or whether it is for a Cancellation/Finish notification, and you would also avoid the race condition and the problem of CallData not being freed at all. Note that, you would need a way to associate your new tag to the call.