ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.73k stars 518 forks source link

ring.util.response/resource-response triggers JDK bug and leaks file descriptors #462

Open pete23 opened 2 years ago

pete23 commented 2 years ago

https://github.com/ring-clojure/ring/blob/master/ring-core/src/ring/util/response.clj#L288 calls .getLastModified which leaks a file descriptor per https://bugs.openjdk.java.net/browse/JDK-8224095

This led to my service failing due to running into the file descriptor ulimit, netty failing to accept new connections, etc. Problem was clear through through /proc/xxx/fd growth and then confirmed with a YourKit memory dump which showed 16K+ Not Closed Files with creation backtraces through connection-last-modified.

Depending on usage pattern and garbage collection this issue may be hidden by the finalizer. However when under load without GC pressure, the file descriptors can be exhausted before this happens.

Workaround: replace use of resource-response with something that bypasses the getLastModified call. I'm considering memoizing all my resource responses (using byte[] bodies) as I have a very small footprint of resources I want to serve.

kommen commented 2 years ago

We're also running into this bug. The more informative JDK bug is: JDK-6956385, as JDK-8224095 is marked as a duplicate of that.

weavejester commented 2 years ago

Given that this bug appears to be fixed in modern versions of the JDK, it doesn't seem worth adding a potentially costly workaround. Perhaps something could be added to the docstring to suggest clojure.java.io/input-stream and clojure.java.io/resource should be used instead for older versions of Java that are not fixed.