clojure-emacs / cider

The Clojure Interactive Development Environment that Rocks for Emacs
https://cider.mx
GNU General Public License v3.0
3.54k stars 645 forks source link

Frequent freezes #534

Closed danielsz closed 10 years ago

danielsz commented 10 years ago

Cider freezes often here.

M-x toggle-debug-on-quit reveals the following:

Debugger entered--Lisp error: (quit)
  accept-process-output(nil 0.005)
  (while (or (null nrepl-sync-response) (null (plist-get nrepl-sync-response :done))) (accept-process-output nil 0.005))
  (save-current-buffer (set-buffer (nrepl-current-connection-buffer)) (setq nrepl-sync-response nil) (nrepl-send-request request (nrepl-sync-request-handler (current-buffer))) (while (or (null nrepl-sync-response) (null (plist-get nrepl-sync-response :done))) (accept-process-output nil 0.005)) nrepl-sync-response)
  nrepl-send-request-sync(("op" "info" "session" "45230ac6-384b-4a1b-acbd-03da3b8187d6" "ns" "twitter-scheduler-client.core" "symbol" "while"))
  (plist-get (nrepl-send-request-sync (list "op" "info" "session" (nrepl-current-session) "ns" (cider-current-ns) "symbol" var)) :value)
  (let ((val (plist-get (nrepl-send-request-sync (list "op" "info" "session" (nrepl-current-session) "ns" (cider-current-ns) "symbol" var)) :value))) (cider--dict-to-alist val))
  (progn (let ((val (plist-get (nrepl-send-request-sync (list "op" "info" "session" (nrepl-current-session) "ns" (cider-current-ns) "symbol" var)) :value))) (cider--dict-to-alist val)))
  (if var (progn (let ((val (plist-get (nrepl-send-request-sync (list "op" "info" "session" ... "ns" ... "symbol" var)) :value))) (cider--dict-to-alist val))))
  cider-var-info("while")
  (assoc attr (cider-var-info var))
  (cadr (assoc attr (cider-var-info var)))
  cider-get-var-attr("while" "arglists")
  cider-eldoc--arglist-op-fn("while")
  (if (nrepl-op-supported-p "info") (cider-eldoc--arglist-op-fn thing) (cider-eldoc--arglist-eval-fn thing))
  cider-eldoc-arglist("while")
  (let* ((info (cider-eldoc-info-in-current-sexp)) (thing (car info)) (pos (cadr info)) (value (cider-eldoc-arglist thing))) (if value (progn (format "%s: %s" (cider-eldoc-format-thing thing) (cider-eldoc-format-arglist value pos)))))
  (progn (let* ((info (cider-eldoc-info-in-current-sexp)) (thing (car info)) (pos (cadr info)) (value (cider-eldoc-arglist thing))) (if value (progn (format "%s: %s" (cider-eldoc-format-thing thing) (cider-eldoc-format-arglist value pos))))))
  (if (cider-connected-p) (progn (let* ((info (cider-eldoc-info-in-current-sexp)) (thing (car info)) (pos (cadr info)) (value (cider-eldoc-arglist thing))) (if value (progn (format "%s: %s" (cider-eldoc-format-thing thing) (cider-eldoc-format-arglist value pos)))))))
  cider-eldoc()
  byte-code("\304 \205?\203\305 !\207\306 \307 \211\204\310\202:  \n@=\2030\311\312\n\"\206:\313  !\202:\313  !\206:\311\312\n\"\305!+\207" [eldoc-documentation-function current-symbol current-fnsym doc eldoc-display-message-p eldoc-message eldoc-current-symbol eldoc-fnsym-in-current-sexp nil apply eldoc-get-fnsym-args-string eldoc-get-var-docstring] 4)
  eldoc-print-current-symbol-info()
  apply(eldoc-print-current-symbol-info nil)
  byte-code("r\301\302H\303H\"\210)\301\207" [timer apply 5 6] 4)
  timer-event-handler([t 0 0 300000 t eldoc-print-current-symbol-info nil idle 0])
bbatsov commented 10 years ago

You're using cider-nrepl, right?

danielsz commented 10 years ago

This is the version string that comes up when I launch cider:

 CIDER 0.6.0alpha (package: 20140419.142) (Java 1.7.0_45, Clojure 1.6.0, nREPL 0.2.3) 
gtrak commented 10 years ago

In the cases where I've seen freezing, there's usually a server-side exception to blame that prevents handlers from getting the response they need, I think it's only an issue in the sync calls.

What would be a sensible default here? Seems like the client-side is not equipped to pass along errors for requests.

Should I rewrite 'cider-var-info' to be async? Seems like a cop-out.

On Wed, Apr 23, 2014 at 8:00 AM, Daniel Szmulewicz <notifications@github.com

wrote:

This is the version string that comes up when I launch cider:

CIDER 0.6.0alpha (package: 20140419.142) (Java 1.7.0_45, Clojure 1.6.0, nREPL 0.2.3)

— Reply to this email directly or view it on GitHubhttps://github.com/clojure-emacs/cider/issues/534#issuecomment-41152472 .

danielsz commented 10 years ago

Here is the server error:

Apr 23, 2014 1:29:13 PM clojure.tools.nrepl.server invoke0
SEVERE: Unhandled REPL handler exception processing message {:id 24, :op info, :session 45230ac6-384b-4a1b-acbd-03da3b8187d6, :ns twitter-scheduler-client.core, :symbol match}
java.lang.NullPointerException
    at sun.misc.MetaIndex.mayContain(MetaIndex.java:243)
    at sun.misc.URLClassPath$JarLoader.getResource(URLClassPath.java:830)
    at sun.misc.URLClassPath.getResource(URLClassPath.java:199)
    at sun.misc.URLClassPath.getResource(URLClassPath.java:251)
    at java.lang.ClassLoader.getBootstrapResource(ClassLoader.java:1305)
    at java.lang.ClassLoader.getResource(ClassLoader.java:1144)
    at java.lang.ClassLoader.getResource(ClassLoader.java:1142)
    at clojure.java.io$resource.invoke(io.clj:441)
    at clojure.java.io$resource.invoke(io.clj:440)
    at cider.nrepl.middleware.info$resource_path.invoke(info.clj:73)
    at cider.nrepl.middleware.info$file_info.invoke(info.clj:92)
    at cider.nrepl.middleware.info$format_response.invoke(info.clj:103)
    at cider.nrepl.middleware.info$info_reply.invoke(info.clj:108)
    at cider.nrepl.middleware.info$wrap_info$fn__18356.invoke(info.clj:116)
    at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__9420.invoke(middleware.clj:17)
    at cider.nrepl.middleware.complete$wrap_complete$fn__18279.invoke(complete.clj:31)
    at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__9420.invoke(middleware.clj:17)
    at cemerick.piggieback$wrap_cljs_repl$fn__10095$fn__10097.invoke(piggieback.clj:255)
    at clojure.lang.AFn.applyToHelper(AFn.java:152)
    at clojure.lang.AFn.applyTo(AFn.java:144)
    at clojure.core$apply.invoke(core.clj:624)
    at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1862)
    at clojure.lang.RestFn.invoke(RestFn.java:425)
    at cemerick.piggieback$wrap_cljs_repl$fn__10095.invoke(piggieback.clj:252)
    at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__9420.invoke(middleware.clj:17)
    at cider.nrepl.middleware.inspect$wrap_inspect$fn__18624.invoke(inspect.clj:77)
    at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__9420.invoke(middleware.clj:17)
    at clojure.tools.nrepl.middleware.session$session$fn__9733.invoke(session.clj:192)
    at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__9420.invoke(middleware.clj:17)
    at clojure.tools.nrepl.server$handle_STAR_.invoke(server.clj:18)
    at clojure.tools.nrepl.server$handle$fn__9796.invoke(server.clj:27)
    at clojure.core$binding_conveyor_fn$fn__4145.invoke(core.clj:1910)
    at clojure.lang.AFn.call(AFn.java:18)
    at java.util.concurrent.FutureTask.run(FutureTask.java:262)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
    at java.lang.Thread.run(Thread.java:744)
danielsz commented 10 years ago

Exceptions in nREPL middleware should not be allowed to freeze the client, right?

gtrak commented 10 years ago

Yea, we should handle the specific issue (NPE in resource-path), and the general one (no sane way to handle exceptions yet from the client).

On Wed, Apr 23, 2014 at 8:56 AM, Daniel Szmulewicz <notifications@github.com

wrote:

Exceptions in nREPL middleware should not be allowed to freeze the client, right?

— Reply to this email directly or view it on GitHubhttps://github.com/clojure-emacs/cider/issues/534#issuecomment-41157007 .

bbatsov commented 10 years ago

@gtrak Unfortunately things like eldoc, completion, etc are inherently sync in nature. That means that adding some same exceptions handling is our only option.

bbatsov commented 10 years ago

I guess @cemerick might have some advice on how best to tackle this.

danielsz commented 10 years ago

I know it's a tough nut to crack. I also know that the client should never freeze. I also also know that some people are giving up on cider because of the usability problems.

It is urgent and critical to stop cider from freezing. Unfortunately, I am not familiar with the codebase, but I'm willing to pair with any of you to work on this.

bbatsov commented 10 years ago

I understand your frustration @danielsz (and that of others), but I've been extremely busy lately and haven't had much time to work on cider. I understand that few Clojure devs know any Emacs Lisp, but once we move more of the code into the middleware layer it will become much easier for Clojure hackers to contribute. Relying on inlined Clojure code was a big big mistake. As the info middleware is the most problematic I guess we can sidestep the problem by catching Exception in it for the time being. @gtrak Any better ideas?

I better idea would probably be to add an error slot to all middleware that can be used synchronously, so we can return immediately in case of problems and let the user know of the problem. But before doing any changes I want to hear @cemerick's opinion.

gtrak commented 10 years ago

Catching exception in info is fine, I think there's a problem in completions now too. This could be centralized with another middleware :-), the details of which might change.

On Wednesday, April 23, 2014, Bozhidar Batsov notifications@github.com wrote:

I understand your frustration @danielsz https://github.com/danielsz(and that of others), but I've been extremely busy lately and haven't had much time to work on cider. I understand that few Clojure devs know any Emacs Lisp, but once we move more of the code into the middleware layer it will become much easier for Clojure hackers to contribute. Relying on inlined Clojure code was a big big mistake. As the info middleware is the most problematic I guess we can sidestep the problem by catching Exceptionin it for the time being. @gtrak https://github.com/gtrak Any better ideas?

I better idea would probably be to add an error slot to all middleware that can be used synchronously, so we can return immediately in case of problems and let the user know of the problem. But before doing any changes I want to hear @cemerick https://github.com/cemerick's opinion.

— Reply to this email directly or view it on GitHubhttps://github.com/clojure-emacs/cider/issues/534#issuecomment-41165064 .

cemerick commented 10 years ago

FYI, I'm travelling this week, so won't immediately have a chance to look into this much.

danielsz commented 10 years ago

Okay, so we know where the pain points are. That's half the job right there. I'm sure @cemerick will provide further guidance, but he's a busy man as well. If you can use some help from me, now, please contact me by email and let's set up a pairing session. Help me get up to speed with navigating and understanding the codebase, and I'll be able to participate in discussions and do actual commits. And incidentally, I do have some emacs lisp knowledge.

PS. Just to be clear, I have nothing but gratitude for the work you've all been doing with cider.

gtrak commented 10 years ago

I'd start on this by looking at test-cases that actually form and perform nrepl requests, I'm not sure where they live, but I suspect the tools.nrepl project would have an example. You'd need to set up the middleware chain just like the plugin. Not sure when I'll get a sufficient chunk of time for a pairing session, but I'd be happy to answer any specific questions, though I'm pretty new to nrepl and cider in general.

On Wed, Apr 23, 2014 at 11:00 AM, Daniel Szmulewicz < notifications@github.com> wrote:

Okay, so we know where the pain points are. That's half the job. I'm sure @cemerick https://github.com/cemerick will provide further guidance, too, but he's a busy man as well. If you can use some help from me, now, please contact me by email and let's set up a pairing session. Help me get up to speed with navigating and understanding the codebase, and I'll be able to participate in discussions and do actual commits. And incidentally, I actually do have some emacs lisp knowledge.

PS. And just to be clear, I have nothing but gratitude for the work you've all been doing with cider.

— Reply to this email directly or view it on GitHubhttps://github.com/clojure-emacs/cider/issues/534#issuecomment-41171524 .

bbatsov commented 10 years ago

In the mean time I've pushed a workaround that should this particular problem for now.

senior commented 10 years ago

I hit this issue as well. Installing cider-nrepl off master - https://github.com/clojure-emacs/cider-nrepl/commit/796849c14339fe59a07655081f88fb7b66723f90 using lein install and bumping my cider-repl dependency to 0.7.0-SNAPSHOT fixed the issue.

I was also able to M-. jump into the function definition that was causing the breakage before.

gtrak commented 10 years ago

This is probably the relevant fix commit: https://github.com/clojure-emacs/cider-nrepl/commit/82e59fa95acf1bc3f7760a5901b63367f8d2f427

On Sat, Apr 26, 2014 at 5:57 PM, Ryan Senior notifications@github.comwrote:

I hit this issue as well. Installing cider-nrepl off master - clojure-emacs/cider-nrepl@796849chttps://github.com/clojure-emacs/cider-nrepl/commit/796849c14339fe59a07655081f88fb7b66723f90using lein install and bumping my cider-repl dependency to 0.7.0-SNAPSHOT fixed the issue.

I was also able to M-. jump into the function definition that was causing the breakage before.

— Reply to this email directly or view it on GitHubhttps://github.com/clojure-emacs/cider/issues/534#issuecomment-41482216 .

cemerick commented 10 years ago

I can't really add anything useful here. It seems like this is fundamentally an implementation detail issue. There was a comment earlier about particular commands being fundamentally synchronous in nature; I presume that that was in reference to emacs commands, as nREPL is fundamentally asynchronous.

gtrak commented 10 years ago

@cemerick, I think the thing I'm really looking for is an example of how to propagate exceptions in the middleware itself to an nrepl client. If there's no preferred method, we could make one up and stick to it in the cider client impl.

On Sat, Apr 26, 2014 at 8:11 PM, Chas Emerick notifications@github.comwrote:

I can't really add anything useful here. It seems like this is fundamentally an implementation detail issue. There was a comment earlier about particular commands being fundamentally synchronous in nature; I presume that that was in reference to emacs commands, as nREPL is fundamentally asynchronous.

— Reply to this email directly or view it on GitHubhttps://github.com/clojure-emacs/cider/issues/534#issuecomment-41484715 .

cemerick commented 10 years ago

Error conditions have to be lifted up into messages just like results are. What that looks like may vary between operation and operation, e.g.:

https://github.com/clojure/tools.nrepl/blob/master/src/main/clojure/clojure/tools/nrepl/middleware/interruptible_eval.clj https://github.com/clojure/tools.nrepl/blob/master/src/main/clojure/clojure/tools/nrepl/middleware/session.clj https://github.com/clojure/tools.nrepl/blob/master/src/main/clojure/clojure/tools/nrepl/middleware/interruptible_eval.clj#L78

chas commented 10 years ago

Gary,

Thinking I'm not the Chas you intended to tag in your post.

Thanks Chas Grundy

Sent from my iPhone

On Apr 26, 2014, at 9:10 PM, Chas Emerick notifications@github.com wrote:

Error conditions have to be lifted up into messages just like results are. What that looks like may vary between operation and operation, e.g.:

https://github.com/clojure/tools.nrepl/blob/master/src/main/clojure/clojure/tools/nrepl/middleware/interruptible_eval.clj https://github.com/clojure/tools.nrepl/blob/master/src/main/clojure/clojure/tools/nrepl/middleware/session.clj

— Reply to this email directly or view it on GitHub.

bbatsov commented 10 years ago

@cemerick Yeah, I was referring to Emacs commands being synchronous in nature (completion, eldoc, etc). I could conjure some callback-based workarounds for them I guess, but I'd rather avoid dealing with this at the moment.

I guess we'll simply stick the stacktraces in the response alongside the results for now and dwell on this more later. :-)

gtrak commented 10 years ago

Weird, github killed my previous comment.

Proof of concept is here: https://github.com/gtrak/cider-nrepl/commit/a230aa267cc248dce5d980848f7993c8b2ae3a81

Would like comments on the elisp bits, and advice on fixing the middleware-dependency-ordering, and a general review of the approach.

Just pushed an update that eliminates the freezing by ensuring :done is sent.

image

bbatsov commented 10 years ago

I'm still on the road. I'll have a look at the code as soon as I'm back from my vacation.

otfrom commented 10 years ago

I think this is the issue I'm having as well. It seems to happen when it tries to do completion activities in the source and the repl buffers. I'm currently tracking what is available on melpa.

gtrak commented 10 years ago

We just pushed a commit that should help if not eliminate the problem while we wait for a general solution. It should already be available from clojars, I think (@bbatsov?), but lein will only check for new snapshots once a day. Delete the cider-nrepl directory in ~/.m2/repository and it'll re-download the next time you start a repl.

On Mon, May 5, 2014 at 5:38 PM, Bruce Durling notifications@github.comwrote:

I think this is the issue I'm having as well. It seems to happen when it tries to do completion activities in the source and the repl buffers. I'm currently tracking what is available on melpa.

— Reply to this email directly or view it on GitHubhttps://github.com/clojure-emacs/cider/issues/534#issuecomment-42244006 .

bbatsov commented 10 years ago

Yep, I've updated cider-nrepl last (European) evening.