Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Background thread did not exit when unsubscribing from watch #482

Closed
ProceduralPeasant opened this issue Oct 21, 2021 · 3 comments
Closed
Labels
api: firestore Issues related to the googleapis/python-firestore API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@ProceduralPeasant
Copy link

Environment details

  • Windows 10:
  • Python version: Python 3.7.9
  • pip version: pip 21.2.4

Steps to reproduce

  1. Create in Firestore a new collection named 'sample' and a 'sample' document inside
  2. Run the example code (it's the exact same snippet code of official documentation)
db = firestore.Client()
doc_ref = db.collection('sample').document('sample')
callback_done = threading.Event()
def on_snapshot(doc_snapshot, changes, read_time):
    for doc in doc_snapshot:
        print(doc.to_dict())
        callback_done.set()
doc_watch = doc_ref.on_snapshot(on_snapshot)
callback_done.wait(10)
doc_watch.unsubscribe()

Error

Background thread did not exit when unsubscribing from watch

@plamut
Copy link

plamut commented Oct 25, 2021

It seems this should be moved to the python-firestore repo.

@plamut plamut transferred this issue from googleapis/python-pubsub Oct 25, 2021
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/python-firestore API. label Oct 25, 2021
@meredithslota meredithslota added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Nov 1, 2021
@tseaver
Copy link
Contributor

tseaver commented Nov 18, 2021

I can reproduce with some tweaks on my Linux system:

$ python3.9 -m venv /tmp/repro-firestore-482

$ cd /tmp/repro-firestore-482/

$ bin/pip install --upgrade setuptools pip 
...
Successfully installed pip-21.3.1 setuptools-59.1.1

$ bin/pip install google-cloud-firestore
...
Successfully installed cachetools-4.2.4 certifi-2021.10.8 charset-normalizer-2.0.7 google-api-core-2.2.2 google-auth-2.3.3 google-cloud-core-2.2.1 google-cloud-firestore-2.3.4 googleapis-common-protos-1.53.0 grpcio-1.42.0 grpcio-status-1.42.0 idna-3.3 packaging-21.3 proto-plus-1.19.8 protobuf-3.19.1 pyasn1-0.4.8 pyasn1-modules-0.2.8 pyparsing-3.0.6 requests-2.26.0 rsa-4.7.2 six-1.16.0 urllib3-1.26.7

$ cat repro_firestore_482.py 
import threading

from google.cloud import firestore

db = firestore.Client()
doc_ref = db.collection('repro-firestore-482').document('sample')
callback_done = threading.Event()

def on_snapshot(doc_snapshot, changes, read_time):

    for doc in doc_snapshot:
        print(doc.to_dict())
        callback_done.set()

doc_watch = doc_ref.on_snapshot(on_snapshot)
doc_ref.set({"sample": "data"})
callback_done.wait(10)
doc_watch.unsubscribe()

$ export GOOGLE_APPLICATION_CREDENTIALS=/path/to/service_account_creds.json

$ bin/python repro_firestore_482.py 
{'sample': 'data'}
Background thread did not exit.

@tseaver
Copy link
Contributor

tseaver commented Nov 18, 2021

I added the following line to configure logging:

logging.basicConfig(level=logging.DEBUG)

Which resulted in this output:

...
{'sample': 'data'}
DEBUG:google.api_core.bidi:waiting for recv.
DEBUG:google.cloud.firestore_v1.watch:Stopping consumer.
INFO:google.cloud.firestore_v1.watch:RPC termination has signaled manager shutdown.
DEBUG:google.api_core.bidi:Cleanly exiting request generator.
DEBUG:google.api_core.bidi:Call to retryable <bound method ResumableBidiRpc._recv of <google.api_core.bidi.ResumableBidiRpc object at 0x7f1fdd70cfa0>> caused <_MultiThreadedRendezvous of RPC that terminated with:
	status = StatusCode.CANCELLED
	details = "Locally cancelled by application!"
	debug_error_string = "None"
>.
DEBUG:google.api_core.bidi:Terminating <bound method ResumableBidiRpc._recv of <google.api_core.bidi.ResumableBidiRpc object at 0x7f1fdd70cfa0>> due to <_MultiThreadedRendezvous of RPC that terminated with:
	status = StatusCode.CANCELLED
	details = "Locally cancelled by application!"
	debug_error_string = "None"
>.
DEBUG:google.api_core.bidi:recved response.
WARNING:google.api_core.bidi:Background thread did not exit.
DEBUG:google.cloud.firestore_v1.watch:Finished stopping manager.

The logging is coming from the BackgroundConsumer.stop method in google/api_core/bidi.py:

    def stop(self):
        """Stop consuming the stream and shutdown the background thread."""
        with self._operational_lock:
            self._bidi_rpc.close()

            if self._thread is not None:
                # Resume the thread to wake it up in case it is sleeping.
                self.resume()
                # The daemonized thread may itself block, so don't wait
                # for it longer than a second.
                self._thread.join(1.0)
                if self._thread.is_alive():  # pragma: NO COVER
                    _LOGGER.warning("Background thread did not exit.")

            self._thread = None

So, it looks to me like we don't have a bug here for firestore: instead, we have a possibly-annoying-but-expected warning emitted by api_core. As it turns out, I added that warning in this PR. As the code notes, the daemonized background thread may in fact be blocked, and so we cannot wait for it forever.

@tseaver tseaver closed this as completed Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/python-firestore API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

4 participants