search

lestrrat-go / httprc

Quasi Up-to-date HTTP In-memory Cache
MIT License
17 stars 5 forks source link

Why does `fetchAndStore` not close the response body? #29

Open 2785 opened 5 months ago

2785 commented 5 months ago

Hey,

I see that the fetchAndStore explicitly has a nolint directive on bodyclose - usage of this in jwx's jwk auto refresh is causing a body close memory leak under some... fairly particular circumstances.

Is the expectation that the transformer should close these bodies?

Cheers

lestrrat commented 5 months ago

The exact reasoning escapes me by now, but I believe it was to somehow allow stackable processing. Something like, peek the body, and depending on the body content, decide on a particular processing pattern from a number of choices.

It's possible that it's a moot point by now.

2785 commented 5 months ago

🤔 I think peek -> process style processing in transformer won't interfere with a simple deferred close in that function.

Well unless the transformer manages to capture the raw reader for an outside scope to access? Seems fairly unintended.

Would you take a PR to add a defer close there (and subsequently pulling this into jwx)?

lestrrat commented 5 months ago

yes, but only if it's opt-in. :)

otherwise it will cause problems to those who rely on the current behavior