InputStream is not closed

[mamciek]

According to Ring Spec if InputStream is given as a :body then it will be consumed an then closed. Same with File. Pedestal consumes InputStream but it does not close it. Proof: I am getting lots of "Too many open files" exceptions using Pedestal and not a signle one when using Ring. It will be nice if Pedestal conforms to the Ring specs as well.

[ohpauleez]

To my knowledge, the close is happening. What version of Pedestal are you using?

[mamciek]

I am using 0.3.0. Can you point me to code that is responsible for closing streams?

[ohpauleez]

Ahh you're talking about the response map, not the incoming request. Are you packaging your response up in a ring-response?

[ohpauleez]

In the same vein, can you point to the Pedestal code you think is causing you issues, or specifically in Ring where the closing operation you're talking about is happening? To my knowledge, that is part of the Ring Response, which Pedestal uses directly.

[mamciek]

I am not using ring-response. I defined simple handler function that returns map with :body (FileInputStream. ...)
I won't point you to Pedestal code, but after quick search I found that it is ring servlet adapter (ring jetty adapter uses it as well) that closes stream.

Ring: https://github.com/ring-clojure/ring/blob/master/ring-servlet/src/ring/util/servlet.clj#L82

[ohpauleez]

Issue would be around: https://github.com/pedestal/pedestal/blob/master/service/src/io/pedestal/http/impl/servlet_interceptor.clj#L118

Either all the protocol implementations need to be responsible for closing (which I'd prefer) or we'd need to do a conditional check to see if the body resource is closeable. I won't be able to get to this until August, but happy to review patches!

Any particular reason why you're not controlling the read and close yourself?

[mamciek]

No reason. Just comparing performance Ring vs Pedestal and found that problem

[ohpauleez]

Ah, cool. Thanks for the report.

---

Be careful when comparing default performance. Pedestal is far-more capable of higher performance than the default settings. I'd suggest: A more specific servlet shim (if it's fitting for you), a custom interceptor stack (only what you need), embrace the async capabilities. In future releases there may be optimized router code (which I have partially done, but I'm still evaluating some design tradeoffs).

In previous tests, the default Pedestal settings were faster than a comparable Ring stack (mainly due to the container integration and the connection code for Jetty).

What Pedestal gives you over Ring: a better security story, async capabilities, easily leverage performant functionality at the container level (rather than handling it in the app).

[ohpauleez]

Fixed in: 8281b80

Thanks for reporting!
Let us know if there's anything else popping up