Potential deadlock in Subscriber

@davidtorres

A lot of our code uses callback without specifying an executor. PollingSubscriberConnection is an example. IIUC, if we don't specify one, the library uses "directExecutor". Is this just the same executor we create the channel with? I think that it is.

If I'm right, this is dangerous. If we receive a bunch of messages, pullers will block on FlowController, waiting for messages to get processed. Messages can't get processed because all the threads in the pool are blocked on FlowController, resulting in a deadlock. I don't think this can happen in default config since we have 5 threads for each puller, but if user provides their own executor, all bets are off.

My current idea: Only use the executor for RPCs. Additionally ask for numWorkerThreads and create a new executor for processing messages. Assuming that the user-provided MessageReceiver cannot directly cause more tasks to be registered to the pullers, we should be able to avoid deadlocks.

What do you think?

what do you mean with use the executor of RPCs?

I think this is WAI, i.e. there is no deadlock. And I might be wrong on how I'm reading the code. But the intent is to use the channel threads to internally process the message, like hold on the flow control, but then dispatch to the "user space" executor for the user to process the message itself and that is the key to prevent a deadlock. And as far I can understand that is how the code is laid out but let me know if I'm misreading it. Perhaps having a unit test that exposes the deadlock will be useful, it should not be that difficult to code if know where the deadlock is happening.

what do you mean with use the executor of RPCs?

I misspoke, I meant the executor we passed to the ManagedChannel. IIUC, we use the same executor to create the channel as the one we use to execute the MessageReceiver code.

I'll see if I can make a test for this. Sounds like a good practice anyway 😆

Decouple the processing of new received messages from the dispatching to