PubSub: Prevent unhandled background error on SPM shutdown#8111
PubSub: Prevent unhandled background error on SPM shutdown#8111plamut merged 1 commit intogoogleapis:masterfrom
Conversation
pubsub/google/cloud/pubsub_v1/subscriber/_protocol/dispatcher.py
Outdated
Show resolved
Hide resolved
|
The main problem here is that the leaser and dispatcher use each other's methods, thus cannot be shut down in a clear linear sequence (and breaking this inter-dependency would likely require substantial refactoring). It's also tricky to synchronize the manager shutdown with any remaining work that dispatcher might have left in its queue (such as pending ACK requests). The thread that initiated the manager shutdown waits on the dispatcher thread to terminate, thus if the latter tries to sync with the manager shutdown through a shared lock, a deadlock can happen. The current fix uses the fact that once in the shutdown mode, several operations on the manager have no effect ( I still need to think of a meaningful yet understandable test to properly cover this. |
|
Could we delay setting the attributes to |
|
@tseaver Definitely, that's exactly the fix that I am just about to submit. Much easier and cleaner than trying to refactor the streaming pull and trying to break the cyclic dependency between the dispatcher and the leaser. |
Closes #6130.
This PR fixes errors in background threads that might happen on user-initiated streaming pull shutdown, i.e. cancelling the future returned by the
subscriber.subscribe()call.How to test
Steps to reproduce:
logging.DEBUG. FWIW, I used the following config during development:Actual result (before the fix):
An error can be observed in
Thread-CallbackRequestDispatcher(the one listed under "Stacktrace 2" in the issue description). The issue is reproducible with high consistency:Expected result (after the fix):
The StreamingPullManager gets shut down cleanly, i.e. without the error reported. There might be a stacktrace of the
Cancelledexception produced byapi_core.bidi, but that is a harmless "expected exception":