Skip to content

fix(integrations): close AsyncDaytona client in DaytonaEnvironment.close()#6307

Open
anxkhn wants to merge 1 commit into
google:mainfrom
anxkhn:fix/daytona-close-client
Open

fix(integrations): close AsyncDaytona client in DaytonaEnvironment.close()#6307
anxkhn wants to merge 1 commit into
google:mainfrom
anxkhn:fix/daytona-close-client

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 5, 2026

Copy link
Copy Markdown

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

  • N/A (no existing issue; described below)

2. Or, if no issue exists, describe the change:

Problem:

DaytonaEnvironment.close() in src/google/adk/integrations/daytona/_daytona_environment.py deletes the sandbox and then drops the AsyncDaytona client by setting self._client = None, without awaiting the client's close():

async def close(self) -> None:
  if self._sandbox is not None:
    await self._sandbox.delete()
    self._sandbox = None
    self._client = None          # client dropped without closing it
    self._is_initialized = False

That client is an AsyncDaytona instance created in _create_sandbox(); it owns the underlying aiohttp ClientSession plus the REST and toolbox API clients. Just dropping the reference never releases those, so a socket/session leak accumulates on every create/close cycle. This matters for long-lived agents and for repeated test runs that set up and tear down an environment.

The Daytona Python SDK documents that AsyncDaytona.close() should be called to properly close the underlying HTTP sessions and avoid resource leaks (or the client can be used as an async context manager). The current code runs none of that.

Solution:

Await self._client.close() before clearing the reference, guarded on the client not being None:

async def close(self) -> None:
  if self._sandbox is not None:
    await self._sandbox.delete()
    self._sandbox = None
    if self._client is not None:
      # Close the AsyncDaytona client to release its underlying HTTP
      # sessions and avoid leaking sockets across create/close cycles.
      await self._client.close()
    self._client = None
    self._is_initialized = False

AsyncDaytona.close() is idempotent (it closes the API clients and the shared aiohttp session, all of which are no-ops when already closed), so the existing "second close() is a no-op" behavior is preserved.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

The existing test_close_deletes_sandbox_and_is_idempotent test used a MagicMock client and never asserted the client was closed, so it did not catch the leak. This PR gives the mock client an AsyncMock close() and asserts DaytonaEnvironment.close() awaits it exactly once (and that it stays a no-op, still awaited only once, on a second close()).

I confirmed the test actually guards the fix: with the production await self._client.close() line removed, the updated test fails with AssertionError: Expected close to have been awaited once. Awaited 0 times.; with the fix in place it passes.

pytest results:

$ pytest tests/unittests/integrations/daytona/test_daytona_environment.py -q
15 passed

No regressions in the wider integrations suite:

$ pytest tests/unittests/integrations/ -q
440 passed

Manual End-to-End (E2E) Tests:

Not run. Exercising this path end to end requires a live Daytona account and API key to provision a real remote sandbox, which is not available in this environment. The change is confined to the client-teardown step of close() and is covered by the unit test above.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end. (Requires a live Daytona account/API key; not available. See note above.)
  • Any dependent changes have been merged and published in downstream modules. (None; no new dependency.)

Additional context

DaytonaEnvironment is a recently added integration (feat(integrations): Add DaytonaEnvironment for remote sandbox workspaces, commit df6baf4a). This is a small follow-up correctness fix to its teardown path.

…ose()

DaytonaEnvironment.close() deleted the sandbox and then dropped the
AsyncDaytona client by setting self._client = None, without awaiting the
client's close(). That client owns the underlying aiohttp session plus
the REST and toolbox API clients, so discarding the reference leaked
sockets on every create/close cycle. The Daytona SDK documents that
AsyncDaytona.close() should be called to release those HTTP sessions and
avoid resource leaks (or the client used as an async context manager).

Await self._client.close() before clearing the reference. close() is
idempotent, so the existing double-close no-op behavior is preserved.

The unit test now gives the mock client an AsyncMock close() and asserts
DaytonaEnvironment.close() awaits it exactly once.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant