Skip to content

Commit

Permalink
Don't step in to clojure.core functions
Browse files Browse the repository at this point in the history
The new step-in command makes it easy to step in (often accidentally) to
a function in clojure.core. But instrumenting clojure core functions is
dangerous, and can result in a stack overflow that crashes the nrepl
process. In fact, just instrumenting `clojure.core/symbol?` (without
even debugging it) causes a stack overflow. I'm not entirely sure yet
how this happens, but it seems safest to just prevent stepping in to all
core functions by default.
  • Loading branch information
grammati committed Apr 14, 2016
1 parent 66dd12b commit 391ba65
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 deletions.
7 changes: 7 additions & 0 deletions src/cider/nrepl/middleware/debug.clj
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,12 @@
(eval form)
(alter-meta! v assoc ::instrumented instrumented)))))))

(defn safe-to-debug?
"Some namespaces are not safe to debug, because doing so can cause a stack
overflow that crashes the nrepl process."
[ns]
(not (#{'clojure.core} (ns-name ns))))

(defn step-in?
"Return true if we can and should step in to the function in the var `v`.
The \"should\" part is determined by the value in `step-in-to-next?`, which
Expand All @@ -464,6 +470,7 @@
;; We do not go so far as to actually try to read the code of the function
;; at this point, which is at macroexpansion time.
(and v
(safe-to-debug? (:ns m))
(not (:macro m))
(not (:inline m))))))

Expand Down
27 changes: 15 additions & 12 deletions test/clj/cider/nrepl/middleware/debug_integration_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -543,30 +543,33 @@
(<-- {:status ["done"]})))

(deftest step-in-to-function-in-jar
;; Step into clojure.core/shuffle. To do this, we need to find and instrument
;; the source, which is in a jar file.
(--> :eval "(ns user.test.step-in)")
;; Step into clojure.tools.nrepl.server/handle*. To do this, we need to find
;; and instrument the source, which is in a jar file. Note that this function
;; is used because it is not marked as a :source-dep, so we can rely on the
;; namespace remaining unmunged, which is important when these tests run on
;; travis CI.
(--> :eval "(ns user.test.step-in
(:require [clojure.tools.nrepl.server :as server]))")
(<-- {:ns "user.test.step-in"})
(<-- {:status ["done"]})

(--> :eval
"#dbg
(defn foo [c]
(shuffle c))")
(defn foo [m]
(server/handle* m identity 23))")
(<-- {:value "#'user.test.step-in/foo"})
(<-- {:status ["done"]})

(--> :eval "(foo (range 2))")
(<-- {:debug-value "(0 1)" :coor [3 1]})
(--> :eval "(foo {})")
(<-- {:debug-value "{}" :coor [3 1]})
(--> :in)

(let [msg (<-- {:debug-value "(0 1)"
:coor [5 1 1 1]})
(let [msg (<-- {:debug-value "{}"
:coor [3 1 1 1]})
file (:file msg)]
(.startsWith file "jar:file:")
(.endsWith file "/clojure/core.clj"))
(.endsWith file "/clojure/tools/nrepl/server.clj"))

(--> :continue)
(let [result (read-string (:value (<-- {})))]
(is (#{[0 1] [1 0]} result)))
(<-- {:value "{:transport 23}"})
(<-- {:status ["done"]}))

0 comments on commit 391ba65

Please sign in to comment.