Memory leak in clojure.tools.deps.alpha/resolve-deps ?

Question

Hi Clojure team,

At Liftoff, we are big users of Clojure (400k LOC across 250+ projects). We have built some tooling that automatically generates deps.edn files for projects, based on static analysis of the project's code.

Some of our tools and test code repeatedly calls clojure.tools.deps.alpha/resolve-deps. As of version v0.14.1178, we have noticed the following:

• resolve-deps creates a new java.util.concurrent.ExecutorService on each call, but doesn't shut it down before returning (code). This lead our test code to spawn 20k+ threads.
• clojure.tools.deps.alpha.util.concurrent/submit-task pushes thread bindings onto a fixed size threadpool, but doesn't pop them (code). As threads are re-used, the frame stack keeps growing.

Internally, we are now using a patched version of the tools that unconditionally shuts down the ExecutorService on each call to resolve-deps and pops the thread bindings for each task. This solves our problem, but we don't know if this is right solution.

How is the core Clojure team thinking about concurrency in the context of the CLI tools? Can we expect this behavior to change in future releases of the CLI tools?

Thanks!

Answer 1

Thanks! These are important problems, and tools.deps should be better behaved for sure. Since a lot of uses of tools.deps are in single-shot use cases (like the CLI), it has mostly not mattered much there, but agree these are issues.

Logged as https://clojure.atlassian.net/browse/TDEPS-227, happy to consider patches if you have them. (see https://clojure.org/dev/dev#_becoming_a_contributor for process).

Comments

Hi Alex. Thanks for your prompt response. I would be happy to contribute patches, but I'm not sure how to sign the Contributor Agreement. My employer has rights with respect to my contributions, but I don't have the signature authority to bind my employer. How should we proceed?

---

Either your employer can sign, or unfortunately we can’t accept your contribution.

Answer 2

Here is a proposed patch:

diff --git a/src/main/clojure/clojure/tools/deps/alpha.clj b/src/main/clojure/clojure/tools/deps/alpha.clj
index ec0561f1..70bef534 100644
--- a/src/main/clojure/clojure/tools/deps/alpha.clj
+++ b/src/main/clojure/clojure/tools/deps/alpha.clj
@@ -489,16 +489,20 @@
   {:arglists '([deps-map args-map])}
   ([deps-map args-map]
    (let [{:keys [extra-deps default-deps override-deps threads trace]} args-map
-         n (or threads concurrent/processors)
-         executor (concurrent/new-executor n)
          deps (merge (:deps deps-map) extra-deps)
-         version-map (-> deps
-                       (canonicalize-deps deps-map)
-                       (expand-deps default-deps override-deps deps-map executor trace)
-                       cut-orphans)
-         lib-map (lib-paths version-map)
-         lib-map' (download-libs executor lib-map deps-map)]
-     (with-meta lib-map' (meta version-map))))
+         n (or threads concurrent/processors)
+         executor (concurrent/new-executor n)]
+     (try
+       (let [version-map (-> deps
+                             (canonicalize-deps deps-map)
+                             (expand-deps default-deps override-deps deps-map executor trace)
+                             cut-orphans)
+             lib-map (lib-paths version-map)
+             lib-map' (download-libs executor lib-map deps-map)]
+         (with-meta lib-map' (meta version-map)))
+       (finally
+         ;; Shutdown executor to avoid creating too many threads.
+         (.shutdownNow executor)))))
   ;; deprecated arity, retained for backwards compatibility
   ([deps-map args-map settings]
    (resolve-deps deps-map (merge args-map settings))))