Skip to content

fix: break CurlHandle reference cycle leaking connections#22

Open
loks0n wants to merge 1 commit into
mainfrom
fix/curl-closure-cycle-leak
Open

fix: break CurlHandle reference cycle leaking connections#22
loks0n wants to merge 1 commit into
mainfrom
fix/curl-closure-cycle-leak

Conversation

@loks0n

@loks0n loks0n commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Problem

Curl::send() registers the header and write callbacks as non-static closures. Closures declared in an instance method bind $this, and curl_setopt stores them on the CurlHandle — which the adapter itself holds:

adapter → CurlHandle → closure → adapter

This reference cycle keeps the handle alive after the last external reference to the Client is dropped, so __destruct (and its curl_close) only runs when PHP's cycle collector fires — by default after 10,000 cycle roots accumulate. Until then, every request from a fresh Client leaks an open keep-alive connection: 2 fds and ~1MB of native TLS buffers per request, invisible to memory_get_usage().

In long-running processes (Swoole workers) doing per-request new Client(), this is catastrophic: we traced an hourly OOMKill of Appwrite Cloud's billing aggregation worker to exactly this — a single worker child held 400+ ESTABLISHED TLS connections and >100MB of native memory mid-job.

Measurements (150 sequential requests, one process)

fds private-dirty RSS
before 309 (grows +2/req) 183MB (grows ~1MB/req)
after (static closures) 9 (flat) 31MB (flat)

Fix

Mark both callbacks static — neither uses $this, so this is behavior-preserving and simply prevents the binding that forms the cycle.

🤖 Generated with Claude Code

The header and write callbacks are non-static closures, so they bind
$this. curl_setopt stores them on the CurlHandle, which the adapter
already holds: adapter -> handle -> closure -> adapter. The cycle keeps
the handle alive past refcount zero, so __destruct never runs until the
cycle collector fires (10k roots) and every request leaks an open
keep-alive connection (~1MB native TLS buffers, 2 fds).

Measured in a long-running Swoole worker doing sequential requests:
150 requests = 309 open fds and 183MB private-dirty RSS; with static
closures: 9 fds and flat memory. Neither callback uses $this.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 26, 2026

Copy link
Copy Markdown

Greptile Summary

Prevents retained cURL connections by making the header and response-body callbacks static, removing their implicit reference to the adapter instance.

Confidence Score: 5/5

The PR appears safe to merge with no actionable defects identified.

The modified callbacks use only parameters and explicitly captured local variables, and the repository's supported PHP versions accept static closures.

Important Files Changed

Filename Overview
src/Adapter/Curl.php Both cURL callbacks are now static; neither accesses instance state, so callback behavior remains unchanged while the CurlHandle reference cycle is removed.

Reviews (1): Last reviewed commit: "fix: break CurlHandle reference cycle le..." | Re-trigger Greptile

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