api7 / api7/ngx_http_ffi_client
Hand-written streaming parser retains the whole response in the request pool
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 2
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
P2 follow-up from the review of #22 (https://github.com/api7/ngx_http_ffi_client/pull/22#issuecomment-5088042507).
In streaming mode the hand-written parser backend (NGX_HTTP_FFI_CLIENT_USE_LLHTTP=0) still accumulates the entire response, which defeats the point of incremental reads.
Detail
ngx_http_ffi_client_response_append() (src/ngx_http_ffi_client_response.c:198-230) appends every received byte into parser->raw, growing it by doubling into the request pool — and an nginx pool never frees the old buffers, so a grown raw costs the sum of all its generations. Consumed bytes are never reclaimed: ngx_http_ffi_client_response_update_body() (:480-510) only advances parser->taken as a cursor into the still-complete buffer.
Result: streaming a 1 GB body or a long-lived SSE stream on the fallback backend holds the whole thing in the request pool until the request ends, even though the caller copied and dropped each chunk. The llhttp backend hands spans out of the read buffer instead (src/ngx_http_ffi_client_response_llhttp.c:384-405), so this is fallback-specific.
Proposal
Either:
- make the fallback streaming path memory-bounded — compact/reset
rawonce a span has been handed out andtakencatches up, so peak usage is bounded by the read buffer plus one pending chunk; or - accept that the hand-written parser is a migration-only fallback (README "Project status" §2) and refuse
streammode on it with a clear error rather than silently buffering everything.
Whichever is chosen should be stated in the README so the two backends' streaming guarantees are not silently different.
Acceptance
- A streaming test on the fallback backend over a body far larger than the read buffer shows bounded memory (or a clean, documented refusal).
- Both backends' streaming semantics are documented.
Related: README "Project status" §2, #12. Part of #18.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with ngx_http_ffi_client_response_append() and ngx_http_ffi_client_response_update_body() in src/ngx_http_ffi_client_response.c, then compare the streaming spans in src/ngx_http_ffi_client_response_llhttp.c. Check the README Project status §2 and related issue #12 before choosing between bounded fallback buffering and refusing stream mode. Done means a fallback streaming test demonstrates bounded memory or a clear refusal, with both backend semantics documented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, nginx
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100