Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support adding -Djdk.attach.allowAttachSelf to jack-in params to enable nrepl JVMTI agent #3693

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Support adding -Djdk.attach.allowAttachSelf to jack-in params to enable nrepl JVMTI agent #3693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### New features

- [#3692](https://github.com/clojure-emacs/cider/pull/3692): Add ability to switch view modes in the ispector (bound to `v`).
- [#3693](https://github.com/clojure-emacs/cider/pull/3693): Add `cider-enable-nrepl-jvmti-agent` defcustom to enable loading native nREPL JVMTI agent which restores thread stop ability on Java 21+.

### Changes

Expand Down
21 changes: 16 additions & 5 deletions cider.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ then concatenated into the \"-M[your-aliases]:cider/nrepl\" form."
:safe #'stringp
:package-version '(cider . "1.1"))


(defcustom cider-clojure-cli-global-aliases
nil
"Global aliases to include when jacking in with the clojure CLI.
Expand All @@ -186,7 +185,6 @@ then concatenated into the \"-M[your-aliases]:cider/nrepl\" form."
:safe #'stringp
:package-version '(cider . "1.14"))


(defcustom cider-shadow-cljs-command
"npx shadow-cljs"
"The command used to execute shadow-cljs.
Expand Down Expand Up @@ -376,6 +374,14 @@ The repl dependendcies are most likely to be nREPL middlewares."
:safe #'booleanp
:version '(cider . "0.11.0"))

(defcustom cider-enable-nrepl-jvmti-agent nil
"When t, add `-Djdk.attach.allowAttachSelf' to the command line arguments.
This allows nREPL JVMTI agent to be loaded. It is needed for evaluation
interruption to properly work on Java 21 and above."
:type 'boolean
:safe #'booleanp
:version '(cider . "1.15.0"))

(defcustom cider-offer-to-open-cljs-app-in-browser t
"When nil, do not offer to open ClojureScript apps in a browser on connect."
:type 'boolean
Expand Down Expand Up @@ -536,7 +542,7 @@ Throws an error if PROJECT-TYPE is unknown."
"List of dependencies where elements are lists of artifact name and version.")
(put 'cider-jack-in-dependencies 'risky-local-variable t)

(defcustom cider-injected-nrepl-version "1.1.2"
(defcustom cider-injected-nrepl-version "1.2.0-beta2"
"The version of nREPL injected on jack-in.
We inject the newest known version of nREPL just in case
your version of Boot or Leiningen is bundling an older one."
Expand Down Expand Up @@ -762,6 +768,8 @@ group:artifact:version notation and MIDDLEWARES are
prepared as arguments to Clojurephant's ClojureNRepl task."
(concat global-opts
(unless (seq-empty-p global-opts) " ")
(when cider-enable-nrepl-jvmti-agent
"-Pjdk.attach.allowAttachSelf ")
(cider--gradle-jack-in-property (append (cider--jack-in-required-dependencies) dependencies))
" "
params
Expand Down Expand Up @@ -813,7 +821,9 @@ removed, LEIN-PLUGINS, LEIN-MIDDLEWARES and finally PARAMS."
(seq-map (lambda (middleware)
(concat "update-in :middleware conj "
middleware))
lein-middlewares))
lein-middlewares)
(when cider-enable-nrepl-jvmti-agent
`(,(concat "update-in :jvm-opts conj -Djdk.attach.allowAttachSelf"))))
" -- ")
" -- "
(if (not cider-enrich-classpath)
Expand Down Expand Up @@ -903,9 +913,10 @@ your aliases contain any mains, the cider/nrepl one will be the one used."
(deps (format "{:deps {%s} :aliases {:cider/nrepl {:main-opts [%s]}}}"
(string-join all-deps " ") main-opts))
(deps-quoted (cider--shell-quote-argument deps command)))
(format "%s-Sdeps %s -M%s:cider/nrepl%s"
(format "%s%s-Sdeps %s -M%s:cider/nrepl%s"
;; TODO: global-options are deprecated and should be removed in CIDER 2.0
(if global-options (format "%s " global-options) "")
(if cider-enable-nrepl-jvmti-agent "-J-Djdk.attach.allowAttachSelf " "")
Copy link
Contributor

@yuhan0 yuhan0 Jun 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late to the review, but are there any practical differences between injecting it this way as a command-line arg vs. adding a :jvm-opts key to the inlined :cider/nrepl alias?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No difference at all. They will not conflict either if set in both places.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean this could be put into the alias instead of a top-level property? Yeah, we could probably do that. Can be changed in the future easily.

Copy link
Contributor

@yuhan0 yuhan0 Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, and since we're now going the route of injecting jvm-opts to clojure-cli jack ins, it might be worth revisiting the decision back in #3023 and see if the -XX:-OmitStackTraceInFastThrow flag could also be added.

deps-quoted
(cider--combined-aliases)
(if params (format " %s" params) ""))))
Expand Down
8 changes: 8 additions & 0 deletions doc/modules/ROOT/pages/basics/up_and_running.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,14 @@ You can further customize the command line CIDER uses for `cider-jack-in` by
modifying the some options. Those differ a bit between the various tools,
so we'll examine them tool by tool.

==== Enabling nREPL JVMTI agent

Since version 1.2.0, nREPL ships together with a native JVMTI agent, so that the
eval interrupts properly work on Java 21 and later. To enable the agent, the
Java process should be launched with `-Djdk.attach.allowAttachSelf`. CIDER will
do it automatically during jack-in if `cider-enable-nrepl-jvmti-agent` is set to
`t`.

==== Leiningen Options

* `cider-lein-command` - the name of the Leiningen executable (`lein` by default)
Expand Down
6 changes: 6 additions & 0 deletions doc/modules/ROOT/pages/usage/code_evaluation.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,12 @@ NOTE: CIDER internally increases the timeout to 30 seconds for the first sync ev

== Configuration

=== Enable evaluation interrupts on Java 21 and newer

The configuration variable `cider-enable-nrepl-jvmti-agent` has to be enabled
for interrupts to work properly on Java 21+. See
xref:basics/up_and_running.adoc#enabling-nrepl-jvmti-agent[JVMTI agent] section.

=== Display Spinner During Evaluation

Some evaluation operations take a while to complete, so CIDER will display a
Expand Down
30 changes: 22 additions & 8 deletions test/cider-tests.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@
(setq-local cider-injected-middleware-version "0.49.0")
(setq-local cider-jack-in-nrepl-middlewares '("cider.nrepl/cider-middleware"))
(setq-local cider-jack-in-dependencies-exclusions '())
(setq-local cider-enrich-classpath t))
(setq-local cider-enrich-classpath t)
(setq-local cider-enable-nrepl-jvmti-agent t))

(it "can inject dependencies in a lein project"
(expect (cider-inject-jack-in-dependencies "" "repl :headless" 'lein)
Expand All @@ -157,6 +158,7 @@
(shell-quote-argument "[cider/cider-nrepl \"0.49.0\"]")
" -- update-in :plugins conj "
(shell-quote-argument "[mx.cider/lein-enrich-classpath \"1.19.3\"]")
" -- update-in :jvm-opts conj -Djdk.attach.allowAttachSelf"
" -- update-in :middleware conj cider.enrich-classpath.plugin-v2/middleware"
" -- repl :headless")))

Expand All @@ -170,6 +172,7 @@
(shell-quote-argument "[cider/cider-nrepl \"0.49.0\"]")
" -- update-in :plugins conj "
(shell-quote-argument "[mx.cider/lein-enrich-classpath \"1.19.3\"]")
" -- update-in :jvm-opts conj -Djdk.attach.allowAttachSelf"
" -- update-in :middleware conj cider.enrich-classpath.plugin-v2/middleware"
" -- repl :headless")))

Expand All @@ -182,6 +185,7 @@
(shell-quote-argument "[cider/cider-nrepl \"0.49.0\"]")
" -- update-in :plugins conj "
(shell-quote-argument "[mx.cider/lein-enrich-classpath \"1.19.3\"]")
" -- update-in :jvm-opts conj -Djdk.attach.allowAttachSelf"
" -- update-in :middleware conj cider.enrich-classpath.plugin-v2/middleware"
" -- repl :headless")))

Expand All @@ -201,6 +205,7 @@
(it "can inject dependencies in a gradle project"
(expect (cider-inject-jack-in-dependencies "--no-daemon" ":clojureRepl" 'gradle)
:to-equal (concat "--no-daemon "
"-Pjdk.attach.allowAttachSelf "
(shell-quote-argument "-Pdev.clojurephant.jack-in.nrepl=nrepl:nrepl:0.9.0,cider:cider-nrepl:0.49.0")
" :clojureRepl "
(shell-quote-argument "--middleware=cider.nrepl/cider-middleware")))))
Expand All @@ -221,6 +226,7 @@
(shell-quote-argument "[cider/cider-nrepl \"0.49.0\"]")
" -- update-in :plugins conj "
(shell-quote-argument "[mx.cider/lein-enrich-classpath \"1.19.3\"]")
" -- update-in :jvm-opts conj -Djdk.attach.allowAttachSelf"
" -- update-in :middleware conj cider.enrich-classpath.plugin-v2/middleware"
" -- repl :headless")))

Expand Down Expand Up @@ -256,6 +262,7 @@
(shell-quote-argument "[cider/cider-nrepl \"0.49.0\"]")
" -- update-in :plugins conj "
(shell-quote-argument "[mx.cider/lein-enrich-classpath \"1.19.3\"]")
" -- update-in :jvm-opts conj -Djdk.attach.allowAttachSelf"
" -- update-in :middleware conj cider.enrich-classpath.plugin-v2/middleware"
" -- repl :headless")))
(it "can concat in a boot project"
Expand All @@ -272,6 +279,7 @@
(it "can concat in a gradle project"
(expect (cider-inject-jack-in-dependencies "--no-daemon" ":clojureRepl" 'gradle)
:to-equal (concat "--no-daemon "
"-Pjdk.attach.allowAttachSelf "
(shell-quote-argument "-Pdev.clojurephant.jack-in.nrepl=nrepl:nrepl:0.9.0,cider:cider-nrepl:0.49.0")
" :clojureRepl "
(shell-quote-argument "--middleware=cider.nrepl/cider-middleware")))))
Expand Down Expand Up @@ -337,6 +345,7 @@
(shell-quote-argument "[cider/cider-nrepl \"0.49.0\"]")
" -- update-in :plugins conj "
(shell-quote-argument "[mx.cider/lein-enrich-classpath \"1.19.3\"]")
" -- update-in :jvm-opts conj -Djdk.attach.allowAttachSelf"
" -- update-in :middleware conj cider.enrich-classpath.plugin-v2/middleware"
" -- repl :headless"))))

Expand Down Expand Up @@ -446,21 +455,22 @@
(it "uses main opts in an alias to prevent other mains from winning"
(setq-local cider-jack-in-dependencies nil)
(setq-local cider-jack-in-nrepl-middlewares '("cider.nrepl/cider-middleware"))
(let ((expected (string-join `("clojure -Sdeps "
(let ((expected (string-join `("clojure -J-Djdk.attach.allowAttachSelf -Sdeps "
,(shell-quote-argument "{:deps {nrepl/nrepl {:mvn/version \"0.9.0\"} cider/cider-nrepl {:mvn/version \"0.49.0\"}} :aliases {:cider/nrepl {:main-opts [\"-m\" \"nrepl.cmdline\" \"--middleware\" \"[cider.nrepl/cider-middleware]\"]}}}")
" -M:cider/nrepl")
"")))
(setq-local cider-allow-jack-in-without-project t)
(setq-local cider-clojure-cli-command "clojure")
(setq-local cider-inject-dependencies-at-jack-in t)
(setq-local cider-clojure-cli-aliases nil)
(setq-local cider-enable-nrepl-jvmti-agent t)
(spy-on 'cider-project-type :and-return-value 'clojure-cli)
(spy-on 'cider-jack-in-resolve-command :and-return-value "clojure")
(expect (plist-get (cider--update-jack-in-cmd nil) :jack-in-cmd)
:to-equal expected)))

(it "allows specifying custom aliases with `cider-clojure-cli-aliases`"
(let ((expected (string-join `("clojure -Sdeps "
(let ((expected (string-join `("clojure -J-Djdk.attach.allowAttachSelf -Sdeps "
,(shell-quote-argument "{:deps {nrepl/nrepl {:mvn/version \"0.9.0\"} cider/cider-nrepl {:mvn/version \"0.49.0\"}} :aliases {:cider/nrepl {:main-opts [\"-m\" \"nrepl.cmdline\" \"--middleware\" \"[cider.nrepl/cider-middleware]\"]}}}")
" -M:dev:test:cider/nrepl")
"")))
Expand All @@ -469,6 +479,7 @@
(setq-local cider-allow-jack-in-without-project t)
(setq-local cider-clojure-cli-command "clojure")
(setq-local cider-inject-dependencies-at-jack-in t)
(setq-local cider-enable-nrepl-jvmti-agent t)
(spy-on 'cider-project-type :and-return-value 'clojure-cli)
(spy-on 'cider-jack-in-resolve-command :and-return-value "clojure")
(expect (plist-get (cider--update-jack-in-cmd nil) :jack-in-cmd)
Expand All @@ -477,7 +488,8 @@
(dolist (command '("clojure" "powershell"))
(it (format "should remove duplicates, yielding the same result (for %S command invocation)" command)
;; repeat the same test for PowerShell too
(let ((expected (string-join `("-Sdeps "
(let ((expected (string-join `("-J-Djdk.attach.allowAttachSelf "
"-Sdeps "
,(cider--shell-quote-argument "{:deps {cider/cider-nrepl {:mvn/version \"0.49.0\"} nrepl/nrepl {:mvn/version \"0.9.0\"}} :aliases {:cider/nrepl {:main-opts [\"-m\" \"nrepl.cmdline\" \"--middleware\" \"[cider.nrepl/cider-middleware]\"]}}}"
command)
" -M:dev:test:cider/nrepl")
Expand All @@ -487,7 +499,8 @@
command)
:to-equal expected))))
(it "handles aliases correctly"
(let ((expected (string-join `("-Sdeps "
(let ((expected (string-join `("-J-Djdk.attach.allowAttachSelf "
"-Sdeps "
,(shell-quote-argument "{:deps {cider/cider-nrepl {:mvn/version \"0.49.0\"} nrepl/nrepl {:mvn/version \"0.9.0\"}} :aliases {:cider/nrepl {:main-opts [\"-m\" \"nrepl.cmdline\" \"--middleware\" \"[cider.nrepl/cider-middleware]\"]}}}")
" -M:test:cider/nrepl")
""))
Expand Down Expand Up @@ -515,25 +528,26 @@
(expect (cider-clojure-cli-jack-in-dependencies nil nil deps)
:to-equal expected)))))
(it "allows for global options"
(let ((expected (string-join `("-J-Djdk.attach.allowAttachSelf -Sdeps "
(let ((expected (string-join `("-J-Xverify:none -J-Djdk.attach.allowAttachSelf -Sdeps "
,(shell-quote-argument "{:deps {cider/cider-nrepl {:mvn/version \"0.49.0\"} nrepl/nrepl {:mvn/version \"0.9.0\"}} :aliases {:cider/nrepl {:main-opts [\"-m\" \"nrepl.cmdline\" \"--middleware\" \"[cider.nrepl/cider-middleware]\"]}}}")
" -M:test:cider/nrepl")
""))
(deps '(("nrepl/nrepl" "0.9.0"))))
(let ((cider-clojure-cli-aliases ":test"))
(expect (cider-clojure-cli-jack-in-dependencies "-J-Djdk.attach.allowAttachSelf" nil deps)
(expect (cider-clojure-cli-jack-in-dependencies "-J-Xverify:none" nil deps)
:to-equal expected))))
(it "allows to specify git coordinate as cider-jack-in-dependency"
(setq-local cider-jack-in-dependencies '(("org.clojure/tools.deps" (("git/sha" . "6ae2b6f71773de7549d7f22759e8b09fec27f0d9")
("git/url" . "https://github.com/clojure/tools.deps/")))))
(let ((expected (string-join `("clojure -Sdeps "
(let ((expected (string-join `("clojure -J-Djdk.attach.allowAttachSelf -Sdeps "
,(shell-quote-argument "{:deps {nrepl/nrepl {:mvn/version \"0.9.0\"} cider/cider-nrepl {:mvn/version \"0.49.0\"} org.clojure/tools.deps { :git/sha \"6ae2b6f71773de7549d7f22759e8b09fec27f0d9\" :git/url \"https://github.com/clojure/tools.deps/\" }} :aliases {:cider/nrepl {:main-opts [\"-m\" \"nrepl.cmdline\" \"--middleware\" \"[cider.nrepl/cider-middleware]\"]}}}")
" -M:cider/nrepl")
"")))
(setq-local cider-allow-jack-in-without-project t)
(setq-local cider-clojure-cli-command "clojure")
(setq-local cider-inject-dependencies-at-jack-in t)
(setq-local cider-clojure-cli-aliases nil)
(setq-local cider-enable-nrepl-jvmti-agent t)
(spy-on 'cider-project-type :and-return-value 'clojure-cli)
(spy-on 'cider-jack-in-resolve-command :and-return-value "clojure")
(expect (plist-get (cider--update-jack-in-cmd nil) :jack-in-cmd)
Expand Down
Loading