Repository: GoogleCloudPlatform/cloud-sql-python-connector (python)

Issue:
Asyncio loop not cleared on closing / garbage collection
## Bug Description

We have a use case where we create connectors on demand from users to connect to Cloud SQL. This implies that we create a `Connector` instance and then `close()` it. After that, the reference to the connector no longer exists so Python GC it.

The issue we see is the exception `OSError`: `[Errno 24] Too many open files` raised at the creation of a connector after a long time. Especially, it is when the asyncio loop is created (see below the stacktrace).

## Example code

```python
with Connector(credentials=credentials) as connector:
    try:
        yield connector.connect(
            self.authentication.instance,
            "pg8000",
            user=self.authentication.username,
            password=self.authentication.password,
            db=self.authentication.database,
        )
        break
    except GoogleAPICallError:
        raise IssueException()
    except ClientResponseError as e:
        if e.status == 429:
            time.sleep(5)
            continue
        else:
            raise IssueException()

```

## Stacktrace
```
OSError: [Errno 24] Too many open files
  File "salescrew/core/celery.py", line 146, in __call__
    raise e
  File "salescrew/core/celery.py", line 143, in __call__
    self.run(*args, **kwargs)
  File "salescrew/connectors/routines.py", line 153, in update_users_from_integrations
    integration_user = connector.get_user(user_integration.id)
  File "salescrew/connectors/databases/postgresql.py", line 297, in get_user
    with self.connection as conn:
  File "contextlib.py", line 135, in __enter__
    return next(self.gen)
  File "salescrew/connectors/databases/postgresql.py", line 346, in connection
    connector = Connector(credentials=credentials)
  File "/usr/local/lib/python3.10/site-packages/google/cloud/sql/connector/connector.py", line 67, in __init__
    self._loop: asyncio.AbstractEventLoop = asyncio.new_event_loop()
  File "asyncio/events.py", line 782, in new_event_loop
    return get_event_loop_policy().new_event_loop()
  File "asyncio/events.py", line 673, in new_event_loop
    return self._loop_factory()
  File "asyncio/unix_events.py", line 64, in __init__
    super().__init__(selector)
  File "asyncio/selector_events.py", line 56, in __init__
    self._make_self_pipe()
  File "asyncio/selector_events.py", line 103, in _make_self_pipe
    self._ssock, self._csock = socket.socketpair()
  File "socket.py", line 607, in socketpair
    a, b = _socket.socketpair(family, type, proto)
```

## How to reproduce

  1. Create a lot of connectors one after another

## Environment

1. OS type and version:  Linux 5.15.55-1-lts
2. Python version: Python 3.10.4
2. Connector version: 0.6.2

## Proposal of resolution

I think because the connector uses the `run_forever` method of the loop, it doesn't clear itself after. Maybe a `self._loop.close()` might need to be done on the closing of the connector ? Or because the loop is used in a thread, we might also need to delete the thread at some point ?


Pull request description:
fix: stop event loop on Connector.close
Background threads used for `Connector()` are daemon threads and by default do not get killed until process exits. Since we are spinning up these threads with `loop.run_forever` the thread will not get killed until the event loop is closed. Which in the current state is when the Python program exits, which is not efficient for long running programs.

To fix this, we should call `loop.stop` when closing a `Connector` object which will in turn kill the background daemon thread.

Closes #409 

Patch:
diff --git a/google/cloud/sql/connector/connector.py b/google/cloud/sql/connector/connector.py
index e656171..064d335 100755
--- a/google/cloud/sql/connector/connector.py
+++ b/google/cloud/sql/connector/connector.py
@@ -284,6 +284,12 @@ class Connector:
         )
         # Will attempt to safely shut down tasks for 5s
         close_future.result(timeout=5)
+        # if background thread exists for Connector, clean it up
+        if self._thread:
+            # stop event loop running in background thread
+            self._loop.call_soon_threadsafe(self._loop.stop)
+            # wait for thread to finish closing (i.e. loop to stop)
+            self._thread.join()
 
     async def close_async(self) -> None:
         """Helper function to cancel Instances' tasks


Test patch:
diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py
index c48787f..554b88d 100644
--- a/tests/unit/test_connector.py
+++ b/tests/unit/test_connector.py
@@ -87,7 +87,6 @@ def test_connect_with_unsupported_driver(connector: Connector) -> None:
         )
     # assert custom error message for unsupported driver is present
     assert exc_info.value.args[0] == "Driver 'bad_driver' is not supported."
-    connector.close()
 
 
 @pytest.mark.asyncio
@@ -157,3 +156,14 @@ async def test_create_async_connector() -> None:
     connector = await create_async_connector()
     assert connector._loop == asyncio.get_running_loop()
     await connector.close_async()
+
+
+def test_Connector_close_kills_thread() -> None:
+    """Test that Connector.close kills background threads."""
+    # open and close Connector object
+    connector = Connector()
+    # verify background thread exists
+    assert connector._thread
+    connector.close()
+    # check that connector thread is no longer running
+    assert connector._thread.is_alive() is False