core: Delay client listener exception notification until normal close

[ejona86]

This should avoid messages being leaked when a Listener throws an exception and
the executor is shut down immediately after the call completes. This is related
to #7105 but a different scenario and we aren't aware of any user having
observed the previous behavior.

Note also this does not fix the similar case of reordering caused by
delayedCancelOnDeadlineExceeded().

CC @njhill

[voidzcy]

LGTM.

Just to make sure I understood the issue correctly:

When the call executor has multiple ClientStreamListener events enqueued, if running one of them that invokes ClientCall.Listener callback throws an exception, ClientCall.Listener.onClose() is invoked and may shut down the call executor. Then the remaining ClientStreamListener events are never executed.

With your change, ClientCall.Listener.onClose() is only invoked when running ClientStreamListener's closed event, which is the last stream callback. So after that, nothing is left in the call executor.

[ejona86]

When the call executor has multiple ClientStreamListener events enqueued, if running one of them that invokes ClientCall.Listener callback throws an exception, ClientCall.Listener.onClose() is invoked and may shut down the call executor. Then the remaining ClientStreamListener events are never executed.

No. In that case the following events are probably executed, because SerializingExecutor will finish draining its queue. The bigger problem is when a throwing event is queued, it throws, and then another event is queued after it throws. After we deliver onClose() we can'd depend on queuing any more events.

[voidzcy]

SerializingExecutor will finish draining its queue. The bigger problem is when a throwing event is queued, it throws, and then another event is queued after it throws.

I see. SerializingExecutor's run() will complete the current batch while there is still an interleave between the two runQueue checks and events being queued after finishing the current batch.