nrepl

https://github.com/nrepl/nrepl || https://nrepl.org
dominicm 2020-06-24T10:28:07.122400Z

Putting this out there in vain hope, has anyone seen an issue where the cider-nrepl middleware causes the JVM to hit it's maximum open files limit? Repro:

clojure -Sdeps '{:deps {nrepl {:mvn/version "0.7.0"} refactor-nrepl {:mvn/version "2.5.0"} cider/piggieback {:mvn/version "0.4.2"} cider/cider-nrepl {:mvn/version "0.25.2"}}}' -m nrepl.cmdline --middleware "[ cider.nrepl/cider-middleware ]"
In another terminal:
# List open file count for the JVM, assumes only one JVM open, tweak as necessary
$ ls -las /proc/$(pgrep 'java')/fd | wc -l
$ clojure -Sdeps '{:deps {nrepl {:mvn/version "0.7.0"}}}' -m nrepl.cmdline -c -p 46397
nREPL 0.7.0
Clojure 1.10.0
OpenJDK 64-Bit Server VM 11.0.5-internal+11-adhoc..jdk11u-jdk-11.0.510
Interrupt: Control+C
Exit:      Control+D or (exit) or (quit)
user=> (+ 1 1)
2
user=> (exit)
$ ls -las /proc/$(pgrep java)/fd | wc -l
4098

dominicm 2020-06-24T10:42:23.122500Z

It appears to be a problem with the debug middleware. I can trim the repro down to typing (require 'cider.nrepl.middleware.debug) which sometimes never returns.

dominicm 2020-06-24T10:46:44.122600Z

OK, tracked it down to (require 'cider.nrepl.inlined-deps.orchard.v0v5v10.orchard.java), so I guess orchard problem

dominicm 2020-06-24T10:58:11.122700Z

It's down to these lines:

(future
  (doseq [class (->> (ns-imports 'clojure.core)
                     (map #(-> % ^Class val .getName symbol)))]
    (class-info class)))

dominicm 2020-06-24T12:39:00.122900Z

Narrowed down to:

(let [klass 'java.lang.Enum]
    (when-let [path (source-path klass)]
      (when-let [root (parse-java path (module-name klass))]
        (.getTypeElement (.getElementUtils root) "java.lang.Enum"))))

bozhidar 2020-06-24T12:48:52.123200Z

> Putting this out there in vain hope, has anyone seen an issue where the cider-nrepl middleware causes the JVM to hit it’s maximum open files limit? Nope. 🙂

bozhidar 2020-06-24T12:51:02.123700Z

Seems you’ve narrowed down the issue pretty quickly, though.

bozhidar 2020-06-24T12:51:59.124200Z

I’m wondering why the resources don’t get released, though.

dominicm 2020-06-24T12:55:07.124300Z

Blugh, sorry, I forgot to mention a critical detail: It's .jar files that are being opened, most importantly, it's my classpath's jar files being duplicated.

dominicm 2020-06-24T12:55:35.124400Z

My open file count nearly doubles when I run the above.

dominicm 2020-06-24T13:06:11.124500Z

In fact, it happens every time I run the above, it adds ~36 items (length of my cp)

dpsutton 2020-06-24T13:06:33.124700Z

adds 36 new open files?

dominicm 2020-06-24T13:10:06.124800Z

Yup, but they're the same files as are already open.

dominicm 2020-06-24T13:10:18.124900Z

So I end up with yada.jar twice, ring.jar twice, etc.

dpsutton 2020-06-24T13:10:35.125200Z

i'm running clojure -Sdeps '{:deps {nrepl {:mvn/version "0.7.0"}}}' -m nrepl.cmdline -c -p 46397 but getting connection refused for some reason

dominicm 2020-06-24T13:13:17.125300Z

@dpsutton you'll need to make sure you use the right port :)

dominicm 2020-06-24T13:13:28.125400Z

But the better way to do all of this is to work from the orchard repo

dominicm 2020-06-24T13:13:34.125500Z

I've detached nrepl itself from the problem now

dpsutton 2020-06-24T13:13:38.125700Z

ah ok

dpsutton 2020-06-24T13:14:27.126400Z

i was thinking that was starting up an nrepl server on port 46397 so the port in the command line was the right port

dominicm 2020-06-24T13:14:36.126500Z

Steps to repro now are to: * Check file descriptor count * Go to orchard, type:

$ clj
Clojure 1.10.1
user=> (require 'orchard.java) (require 'orchard.java.parser) (in-ns 'orchard.java.parser)
nil
nil
#object[clojure.lang.Namespace 0x5c83ae01 "orchard.java.parser"]
(let [klass 'java.lang.Enum]
    (when-let [path (source-path klass)]
      (when-let [root (parse-java path (module-name klass))]
        (.getTypeElement (.getElementUtils root) (str klass)))))
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by clojure.lang.InjectedInvoker/0x0000000800232040 (file:/home/overfl0w/.m2/repository/org/clojure/clojure/1.10.1/clojure-1.10.1.jar) to method com.sun.tools.javac.model.JavacElements.getTypeElement(java.lang.CharSequence)
WARNING: Please consider reporting this to the maintainers of clojure.lang.InjectedInvoker/0x0000000800232040
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
#object[com.sun.tools.javac.code.Symbol$ClassSymbol 0x383caf89 "java.lang.Enum"]
* Check file descriptor again

dominicm 2020-06-24T13:14:45.126700Z

Sorry, no :p I was a bit lazy

dominicm 2020-06-24T13:15:39.126800Z

I'm starting to think that I'm going to end up reporting a JDK bug :(

dominicm 2020-06-24T13:17:15.127Z

find /proc/$(pgrep java)/fd -printf '%l\n' | sort | uniq --repeated will show you what's repeated in the fds.

dominicm 2020-06-24T13:22:34.127200Z

If I reuse the return value of getElementUtils, it doesn't use additional. So it's a resource managed by getElementUtils, but activated by calling getTypeElement.

dpsutton 2020-06-24T13:23:44.127800Z

well speaking of jdk bug, can you easily switch versions and repo, and then switch jdk vendors and repo? got correto laying around?

dominicm 2020-06-24T13:27:28.127900Z

I can switch to 8, but that uses a totally different code path, so doesn't seem worth testing.

dominicm 2020-06-24T13:27:42.128Z

I'll give corretto a go

dominicm 2020-06-24T13:32:01.128100Z

I can repro on corretto 11.

dominicm 2020-06-24T13:38:37.128300Z

If I close the JavaFileManager under the env, the files are cleaned up!

dominicm 2020-06-24T13:42:27.128400Z

Sticking a finally around the use of the doclet env which closes the file manager seems to do the trick, but I'd love to find some java docs saying what the cleanup is for this kind of thing…

dominicm 2020-06-24T13:42:50.128500Z

Alternatively, we could use our own file manager, which might dedupe if it's (re)used across doclets. Then you only double the classpath.

dpsutton 2020-06-24T13:47:20.128700Z

where is this?

dominicm 2020-06-24T13:47:59.128800Z

In the orchard repo, in orchard.java.parser, this version fixes it:

(defn source-info
  "If the source for the Java class is available on the classpath, parse it
  and return info to supplement reflection. Specifically, this includes source
  file and position, docstring, and argument name info. Info returned has the
  same structure as that of `orchard.java/reflect-info`."
  [klass]
  {:pre [(symbol? klass)]}
  (try
    (when-let [path (source-path klass)]
      (when-let [root (parse-java path (module-name klass))]
        (try
          (assoc (->> (.getIncludedElements ^DocletEnvironment root)
                      (filter #(#{ElementKind/CLASS
                                  ElementKind/INTERFACE
                                  ElementKind/ENUM}
                                 (.getKind ^Element %)))
                      (map #(parse-info % root))
                      (filter #(= klass (:class %)))
                      (first))
                 :file path
                 :path (.getPath (io/resource path)))
          (finally
            (.close (.getJavaFileManager root))))))
    (catch Throwable _)))

dominicm 2020-06-24T13:48:05.128900Z

I think I've found the info I was looking for

dominicm 2020-06-24T13:50:08.129Z

https://docs.oracle.com/en/java/javase/11/docs/api/java.compiler/javax/tools/JavaCompiler.html > Reusing a file manager can potentially reduce overhead of scanning the file system and reading jar files. Although there might be no reduction in overhead, a standard file manager must work with multiple sequential compilations making the following example a recommended coding pattern: And the recommended coding pattern includes: > fileManager.close();

dominicm 2020-06-24T13:50:45.129100Z

Given that we want to dynamically work with new docs over an extended period, I think the correct behavior is to reuse a file manager

dominicm 2020-06-24T13:56:48.129200Z

Reusing the file manager seems to have done the trick. It doesn't even double the count, it just doesn't go up at all!

dominicm 2020-06-24T13:57:33.129300Z

oh, that's because it's broken and there's a (catch Throwable _) 😁

dominicm 2020-06-24T13:58:34.129400Z

Okay, now it behaved like I expect. It went 36->67, but it didn't keep ballooning like it did before. I forgot to note that I commented out the (future).

dominicm 2020-06-24T14:51:12.129500Z

So, it works from the REPL, but not the tests.

dominicm 2020-06-24T15:01:44.129600Z

And they run fine independently. Yay. State.

dominicm 2020-06-24T15:16:49.129700Z

This is why it fails in the tests (and nowhere else):

/tmp/ISeq.java:18: error: cannot find symbol
public interface ISeq extends IPersistentCollection {
                              ^
  symbol: class IPersistentCollection
1 error
I don't really understand :(

dominicm 2020-06-24T15:20:26.129800Z

I wonder if there's something stateful going on with the file manager...

dominicm 2020-06-24T16:05:43.130300Z

I can't get compiler reuse version to work, so I'm going to assume that it can't work for some technical reason I don't understand, and instead use .close.

dominicm 2020-06-24T16:06:02.130400Z

It's no less efficient than the code is now, it just would have been faster if I'd got it to work

dominicm 2020-06-24T16:06:11.130500Z

I suspect it's because there's compiler state between builds, or something.

dominicm 2020-06-24T16:19:08.130600Z

https://github.com/clojure-emacs/orchard/pull/95 Opened the .close solution.

dominicm 2020-06-24T16:19:30.130800Z

I guess I can try and workaround this by monkeypatching cider's orchard? Yikes.