Unused destructured local not cleared, causes memory leak
Description
Environment
None
Attachments
2
- 03 Jun 2015, 07:46 PM
- 03 Jun 2015, 04:17 PM
Activity
Show:
Nicola MomettoDecember 15, 2015 at 5:47 PM
While investigating an unrelated memory leak on core.async (ASYNC-138) I discovered that this bug also affects code inside a `go` macro, since it often emits unreachable bindings
clojure.core.async> (macroexpand '(go (let [test (range 1e10)]
(str test (<! (chan))))))
(let*
[c__15123__auto__ (clojure.core.async/chan 1) captured-bindings__15124__auto__ (clojure.lang.Var/getThreadBindingFrame)]
(clojure.core.async.impl.dispatch/run
(fn*
[]
(clojure.core/let
[f__15125__auto__
(clojure.core/fn
state-machine__14945__auto__
([] (clojure.core.async.impl.ioc-macros/aset-all! (java.util.concurrent.atomic.AtomicReferenceArray. 9) 0 state-machine__14945__auto__ 1 1))
([state_17532]
(clojure.core/let
[old-frame__14946__auto__
(clojure.lang.Var/getThreadBindingFrame)
ret-value__14947__auto__
(try
(clojure.lang.Var/resetThreadBindingFrame (clojure.core.async.impl.ioc-macros/aget-object state_17532 3))
(clojure.core/loop
[]
(clojure.core/let
[result__14948__auto__
(clojure.core/case
(clojure.core/int (clojure.core.async.impl.ioc-macros/aget-object state_17532 1))
1
(clojure.core/let
[inst_17525 (clojure.core.async.impl.ioc-macros/aget-object state_17532 7) inst_17525 (range 1.0E10) test inst_17525 inst_17526 str test inst_17525 inst_17527 (chan) state_17532 (clojure.core.async.impl.ioc-macros/aset-all! state_17532 7 inst_17525 8 inst_17526)]
(clojure.core.async.impl.ioc-macros/take! state_17532 2 inst_17527))
2
(clojure.core/let
[inst_17525 (clojure.core.async.impl.ioc-macros/aget-object state_17532 7) inst_17526 (clojure.core.async.impl.ioc-macros/aget-object state_17532 8) inst_17529 (clojure.core.async.impl.ioc-macros/aget-object state_17532 2) inst_17530 (inst_17526 inst_17525 inst_17529)]
(clojure.core.async.impl.ioc-macros/return-chan state_17532 inst_17530)))]
(if (clojure.core/identical? result__14948__auto__ :recur) (recur) result__14948__auto__)))
(catch java.lang.Throwable ex__14949__auto__ (clojure.core.async.impl.ioc-macros/aset-all! state_17532 clojure.core.async.impl.ioc-macros/CURRENT-EXCEPTION ex__14949__auto__) (clojure.core.async.impl.ioc-macros/process-exception state_17532) :recur)
(finally (clojure.lang.Var/resetThreadBindingFrame old-frame__14946__auto__)))]
(if (clojure.core/identical? ret-value__14947__auto__ :recur) (recur state_17532) ret-value__14947__auto__))))
state__15126__auto__
(clojure.core/-> (f__15125__auto__) (clojure.core.async.impl.ioc-macros/aset-all! clojure.core.async.impl.ioc-macros/USER-START-IDX c__15123__auto__ clojure.core.async.impl.ioc-macros/BINDINGS-IDX captured-bindings__15124__auto__))]
(clojure.core.async.impl.ioc-macros/run-state-machine-wrapped state__15126__auto__))))
c__15123__auto__)importNovember 18, 2015 at 12:12 PM
Comment made by: jarohen
FYI - we've hit this as a memory leak in our production system:
(defn write-response! [{:keys [products merchant-id] :as search-result} writer response-type]
;; not using `search-result` throughout this fn - kept in to document intent
;; hangs on to `products`, a large lazy-seq, until it's completely consumed, causes memory leak
...
(case response-type
"application/edn" (print-method products writer)
...))(defn write-response! [{:keys [products merchant-id]} writer response-type]
;; fine, releases earlier elements in products as it flies through the response
...
(case response-type
"application/edn" (print-method products writer)
...))The work-around in our case is easy enough - removing the unused symbol - but, given we assumed including an unused symbol would be a no-op, it did take us a while to find!
Cheers,
James
Michael BlumeJune 3, 2015 at 6:57 PM
Nice =)
Completed
Details
Assignee
UnassignedUnassignedReporter
Nicola MomettoNicola MomettoApproval
OkPatch
CodePriority
CriticalFix versions
Details
Details
Assignee
Unassigned
UnassignedReporter
Nicola Mometto
Nicola MomettoApproval
Ok
Patch
Code
Priority
CriticalFix versions
Created June 3, 2015 at 4:16 PM
Updated August 19, 2016 at 5:34 PM
Resolved August 19, 2016 at 5:34 PM
Clojure currently doesn't clear unused locals. This is problematic as some form of destructuring can generate unused/unusable locals that the compiler cannot clear and thus can cause head retention:
;; this works user=> (loop [xs (repeatedly 2 #(byte-array (quot (.maxMemory (Runtime/getRuntime)) 10)))] (when (seq xs) (recur (rest xs)))) nil ;; this doesn't user=> (loop [[x & xs] (repeatedly 200 #(byte-array (quot (.maxMemory (Runtime/getRuntime)) 10)))] (when (seq xs) (recur xs))) OutOfMemoryError Java heap space clojure.lang.Numbers.byte_array (Numbers.java:1252)Here's a macroexpansion that exposes this issue:
user=> (macroexpand-all '(loop [[a & b] c] [a b])) (let* [G__21 c vec__22 G__21 a (clojure.core/nth vec__22 0 nil) b (clojure.core/nthnext vec__22 1)] (loop* [G__21 G__21] (let* [vec__23 G__21 a (clojure.core/nth vec__23 0 nil) b (clojure.core/nthnext vec__23 1)] [a b])Cause: The first two bindings of a and b will hold onto the head of c since they are never used and not accessible from the loop body they cannot be cleared.
Approach: Track whether local bindings are used. After evaluating the binding expression, if the local binding is not used and can be cleared, then pop the result rather than storing it.
Patch: 0001-CLJ-1744-clear-unused-locals-v2.patch
Screened by: Alex Miller