Skip to content

fix(fireworks): retain root client refs to prevent aiohttp session leak#36202

Open
Mason Daugherty (mdrxy) wants to merge 1 commit intomasterfrom
mason/fix-fireworks-client-session-leak
Open

fix(fireworks): retain root client refs to prevent aiohttp session leak#36202
Mason Daugherty (mdrxy) wants to merge 1 commit intomasterfrom
mason/fix-fireworks-client-session-leak

Conversation

@mdrxy
Copy link
Copy Markdown
Member

@mdrxy Mason Daugherty (mdrxy) commented Mar 24, 2026

Summary

  • ChatFireworks.validate_environment creates Fireworks / AsyncFireworks instances, extracts .chat.completions, and immediately discards the parents
  • Starting with fireworks-ai>=0.19, FireworksClient.__init__ eagerly creates an aiohttp.ClientSession when an async event loop is running
  • Orphaning the parents means these sessions are never closed, producing Unclosed client session warnings on stderr
  • Stores parent instances as root_client / root_async_client (matching langchain-openai pattern) so transports stay reachable and are cleaned up by GC

Reproduction

In an async context (e.g., LangGraph agent, Harbor eval runner):

import asyncio, gc, warnings
warnings.simplefilter("always")

async def main():
    with warnings.catch_warnings(record=True) as caught:
        warnings.simplefilter("always")
        from langchain_fireworks import ChatFireworks
        model = ChatFireworks(model="accounts/fireworks/models/llama-v3p1-8b-instruct", api_key="test-key")
        del model
        gc.collect()
        await asyncio.sleep(0)
        gc.collect()

    unclosed = [w for w in caught if issubclass(w.category, ResourceWarning) and "unclosed" in str(w.message).lower()]
    print(f"{len(unclosed)} leaked sessions")  # 4 before fix, 0 after

asyncio.run(main())

Additional context

  • The fireworks-ai sync close() also doesn't clean up the aiohttp session created in async contexts — separate upstream issue
  • The regression test will only catch the leak once the fireworks-ai pin is bumped to >=0.19; on 0.15.x the aiohttp session isn't created so the test passes vacuously

`ChatFireworks.validate_environment` extracted `.chat.completions` from
freshly created `Fireworks` / `AsyncFireworks` instances and immediately
discarded the parents.  Starting with `fireworks-ai>=0.19`, those parents
eagerly create an `aiohttp.ClientSession` when an async event loop is
running.  Orphaning the parents means the sessions are never closed,
producing "Unclosed client session" warnings at interpreter shutdown.

Store parent instances as `root_client` / `root_async_client` (matching
the `langchain-openai` pattern) so transports stay reachable and are
cleaned up deterministically.

Adds a regression test that constructs `ChatFireworks` inside an async
context and asserts no `ResourceWarning` about unclosed sessions fires.
@github-actions github-actions bot added fireworks `langchain-fireworks` package issues & PRs fix For PRs that implement a fix integration PR made that is related to a provider partner package integration internal size: S 50-199 LOC labels Mar 24, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 24, 2026

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 1 untouched benchmark
⏩ 39 skipped benchmarks1


Comparing mason/fix-fireworks-client-session-leak (d50251e) with master (d22df94)

Open in CodSpeed

Footnotes

  1. 39 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fireworks `langchain-fireworks` package issues & PRs fix For PRs that implement a fix integration PR made that is related to a provider partner package integration internal size: S 50-199 LOC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant