<< Back to previous view

[NREPL-17] Agent nesting causes issues Created: 13/Apr/12  Updated: 16/Apr/12  Resolved: 16/Apr/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Defect Priority: Blocker
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

We talked about this briefly on IRC/github: https://github.com/overtone/overtone/issues/85. I guess the nicest way to fix it would be to stop using agents in nREPL. There's only one spot where agents are created, and a few send-off calls (no sends), all internal, so any solution will be non-intrusive from an API perspective.

One implementation idea is to use a simple blocking queue and thread pool in java-land.

Another is to use nearly the same implementation as Agent, but remove the nesting restriction (call it NestableAgent?). Also get rid of the STM interaction and validator business since those aren't available outside the clojure.lang package - and then provide a parallel version of send-off. I've spiked a bit on this one, but I'm starting to think a plain queue/thread pool is cleaner.

Thoughts?



 Comments   
Comment by Chas Emerick [ 13/Apr/12 5:18 AM ]

Yes, I started working on this yesterday. Agents have definitely proven themselves unsuitable here, and a poor choice.

I'm not going to be fancy; we don't need fancy here.

Comment by Colin Jones [ 13/Apr/12 5:49 AM ]

Great news! Thanks for the quick response.

Comment by Chas Emerick [ 16/Apr/12 3:08 PM ]

Fixed in 0.2.0-beta6.





[NREPL-25] Metadata -> middleware/handler documentation Created: 10/Aug/12  Updated: 13/Aug/12  Resolved: 13/Aug/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0-beta9

Type: Enhancement Priority: Blocker
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by Chas Emerick [ 13/Aug/12 11:31 PM ]

See clojure.tools.nrepl.middleware/wrap-describe, set-descriptor!, etc.





[NREPL-32] Handling of messages is serialized on a per-connection basis Created: 03/Oct/12  Updated: 04/Oct/12  Resolved: 04/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0-beta10

Type: Defect Priority: Blocker
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by Chas Emerick [ 04/Oct/12 12:31 AM ]

Fixed @ 2f12262834e5b93b8536c81b12df25b2b75e0254.





[NREPL-26] Middleware metadata + collection of middleware names => well-formed handler Created: 10/Aug/12  Updated: 20/Aug/12  Resolved: 20/Aug/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0-beta9

Type: Enhancement Priority: Blocker
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by Chas Emerick [ 20/Aug/12 3:48 PM ]

See clojure.tools.nrepl.middleware, the metadata descriptors described there, etc.





[NREPL-49] Chas, where are you? You said you were going to the Hops & Hominy restaurant Created: 25/Mar/14  Updated: 28/Mar/14  Resolved: 28/Mar/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Blocker
Reporter: Andy Fingerhut Assignee: Chas Emerick
Resolution: Not Reproducible Votes: 0
Labels: bug
Environment:

San Francisco


Attachments: JPEG File IMG_20140326_174912.jpg    

 Description   

You said you, Gary, and Chris were going to meet me at Hops & Hominy restaurant after Clojure/West. I am here, but I don't see you guys anywhere.

Steps to reproduce: Walk around the restaurant, looking for you at every table. You are not there.



 Comments   
Comment by Chas Emerick [ 26/Mar/14 2:13 PM ]

This is officially the most awesome JIRA ticket, ever. <3 Andy. :-D

Comment by Chas Emerick [ 26/Mar/14 2:45 PM ]

People keep asking, so: yes, we got there and all had a good time.

Comment by Justin Balthrop [ 26/Mar/14 7:40 PM ]

Chas, I'm having a similar issue. You said you'd be around, but I can't find you.

Comment by Aaron Brooks [ 26/Mar/14 7:55 PM ]

I've re-run the tests and found Chas to be in the Garden Court of the Palace Hotel. See the attached test results.

Comment by Chas Emerick [ 28/Mar/14 12:07 PM ]

@Justin, this is likely a race condition. I was there (as indicated by the test results, thanks Aaron), but you weren't. If Reid had finished his test.check scheduler, we could have prevented a buggy interleaving. Since we've all decamped from SF at this point, I don't know that we'll be able to properly reach a resolution.

I'm closing this as 'not reproducible' for now.





[NREPL-12] Intermittent test failures for stdin Created: 26/Feb/12  Updated: 10/Jun/12  Resolved: 10/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Defect Priority: Critical
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Improve-timing-issues-in-tests.patch    

 Description   

Failures like these are happening sometimes in the test suite, mainly around stdin:

FAIL in (request-multiple-read-objects-in) (nrepl_test.clj:211)
expected: (= (quote (:ohai)) (response-values (for [resp (repl-eval session "(read)")] (do (when (-> resp :status set (contains? "need-input")) (session {:op :stdin, :stdin ":ohai :kthxbai\n"})) resp))))
actual: (not (= (:ohai) nil))

FAIL in (request-multiple-read-objects-in) (nrepl_test.clj:217)
expected: (= [" :kthxbai"] (repl-values session "(read-line)"))
actual: (not (= [" :kthxbai"] nil))

I've done a ton of debugging and thinking about this, and haven't gotten far. A few theories I had that seemed decent at the time, but ultimately haven't gotten rid of these occasional failures:

The debugging I've been doing (print statements galore) suggests that when this problem happens, the client does send back the proper :stdin, but it doesn't get picked up by the add-stdin handler in time to write to the :stdin-writer and avoid the expiring timeout.

I'm pretty much stumped at this point, unfortunately.



 Comments   
Comment by Chas Emerick [ 29/Mar/12 4:18 PM ]

This is becoming a serious problem; the build fails half the time on build.clojure.org, making it incredibly frustrating to perform releases, etc.

Any new ideas / evaluation of this is most welcome.

Comment by Colin Jones [ 15/Apr/12 10:24 PM ]

My current vague suspicion is that it's a CPU contention thing between the server and client.

It's kind of hard for me to reason about the execution of the response-values lazy seq, and where the blocking actually happens. And of course adding logging changes the behavior sometimes. I wouldn't be opposed to adding sleeps in there, but I expect those kinds of things would still be flaky later, on build machines in particular :/

Incidentally, bumping the client timeout up to Long.MAX_VALUE helps a bit the stdin failures, but doesn't solve them. It does create new ones: session-lifecycle, unknown-op, and read-timeout block forever trying to reduce across the whole seq in combine-responses. It was a bit surprising to me to see that reduce in the response-values pipeline, and I pursued that at one point as well, but it hasn't gotten me anywhere. Decreasing the timeout helps to reproduce the failures.

I got this thread dump from a blocking-forever stdin test (run via `lein test clojure.tools.nrepl-test`): https://refheap.com/paste/2141.

Comment by Colin Jones [ 03/Jun/12 5:07 AM ]

With this patch, I was able to run clojure.tools.nrepl-test 100 times successfully. Previous attempts would generally only get as far as 15 or so before failing for one reason or another.

It also gets rid of a race condition with my skip-if-eol stuff that was addressing multiple input reads. Lastly, it avoids the need for the 10-second timeout, so the tests are faster now as well.

Comment by Chas Emerick [ 09/Jun/12 10:57 AM ]

I'm psyched that you may have found the solution to these problems. I've applied the patch locally, and it tests well. I want to eyeball it for a minute (probably tomorrow) before moving on.

Comment by Chas Emerick [ 10/Jun/12 4:40 PM ]

Pushed, will be in the next beta release. Thanks so much!





[NREPL-31] REPL utilities are refered into *ns* prior to every expression evaluation Created: 01/Oct/12  Updated: 22/Oct/12  Resolved: 22/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta9
Fix Version/s: 0.2.0, 0.2.0-RC1

Type: Defect Priority: Critical
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Declined Votes: 0
Labels: None


 Description   

The refers of REPL utilities (pst, pprint, etc) happens in clojure.main/repl. In a terminal/single-threaded REPL, this is called just once, so it only ever affects the user namespace. Every expression sent for evaluation by nREPL invokes clojure.main/repl though, so moving *ns* around will inadvertently cause those refers to happen over and over, into non-user namespaces. (I've been enjoying having pprint and pp available all the time, but I'd never thought much about why they were always there.)

In the end, this is an nREPL bug.

I don't see any easy way out off the top of my head. I think nREPL will end up having to stop using clojure.main/repl, and maintain a modified version of it itself (something I wanted to avoid exactly so as to benefit from the changes to clojure.main/repl from version to version of Clojure).

Suggestions most welcome.

(Originally reported here.)



 Comments   
Comment by Chas Emerick [ 11/Oct/12 2:10 PM ]

Hopefully CLJ-1085 is resolved and this can drop off. In the meantime, not going to hold off -beta10 for it or this.

Comment by Chas Emerick [ 22/Oct/12 8:01 PM ]

Fixed upstream in CLJ-1085. Note that this issue will continue to affect those using Clojure < 1.5.0.





[NREPL-2] send-ack probably leaks connections/sockets Created: 30/Sep/11  Updated: 21/Feb/12  Resolved: 21/Feb/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Defect Priority: Critical
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

The let in send-ack should be be with-open.






[NREPL-19] Android: nREPL starts with no namespace Created: 11/May/12  Updated: 20/Jun/12  Resolved: 20/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alexander Yakushev Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: android,, bug
Environment:

Android w/ Clojure 1.4.0, REPLy client / Eclipse CCW client



 Description   

When I start the nREPL on the Android device (by calling `(clojure.tools.nrepl.server/start-server :port 9999)`) it all goes well. But when I try to connect to this REPL using any of the clients I find myself in an empty namespace (or something like that). The var ns is unbound, no functions from the clojure.core are available. At the beginning REPLy tries to perform some actions but they fail (says that it cannot find symbol `defn` - because nothing from clojure.core is being mapped).

However I can do (in-ns 'anywhere) it works. Everything else in the REPL works correctly (as far as I see). The issue itself is minor but I'm afraid that it is caused by some crash during nREPL initialization that might lead to other problems in future.



 Comments   
Comment by Alexander Yakushev [ 11/May/12 11:06 AM ]

Can't edit, I meant the *ns* var, of course.

Comment by Alexander Yakushev [ 19/May/12 4:32 PM ]

With the help of Daniel Solano Gómez I managed to fix this bug. The problem was caused by the lack of user namespace in the Android-patched Clojure. nREPL assumes that the user namespace is present and uses it by default.

Here's the so called fix I ended up with:

...
(let [user-ns (create-ns 'user)]
  (binding [*ns* user-ns]
    (clojure.tools.nrepl.server/start-server :port 9999)))
...

The issue can be closed now.

Comment by Chas Emerick [ 27/May/12 4:04 PM ]

A follow up Q: user is created by clojure.lang.RT's static initialization. Is the lack of that in "Android-patched Clojure" an optimization of some sort?

Comment by Alexander Yakushev [ 27/May/12 4:49 PM ]

Exactly, Daniel Solano Gómez removed it because it's initialization took additional time, it seems.





[NREPL-23] NPE when interruptible-eval/evaluate is passed a non-existant namespace Created: 22/Jul/12  Updated: 20/Aug/12  Resolved: 20/Aug/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: 0.2.0-beta9

Type: Defect Priority: Major
Reporter: Joe Snikeris Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   
Stack Trace
Exception in thread "nREPL-worker-2" java.lang.NullPointerException
	at clojure.core$refer.doInvoke(core.clj:3779)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invoke(core.clj:603)
	at clojure.core$load_lib.doInvoke(core.clj:5279)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invoke(core.clj:603)
	at clojure.core$load_libs.doInvoke(core.clj:5298)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invoke(core.clj:605)
	at clojure.core$use.doInvoke(core.clj:5392)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.main$repl.doInvoke(main.clj:259)
	at clojure.lang.RestFn.invoke(RestFn.java:1096)
	at clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__337.invoke(interruptible_eval.clj:51)
	at clojure.lang.AFn.applyToHelper(AFn.java:159)
	at clojure.lang.AFn.applyTo(AFn.java:151)
	at clojure.core$apply.invoke(core.clj:601)
	at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1771)
	at clojure.lang.RestFn.invoke(RestFn.java:425)
	at clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke(interruptible_eval.clj:36)
	at clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__374$fn__376.invoke(interruptible_eval.clj:162)
	at clojure.core$comp$fn__4034.invoke(core.clj:2278)
	at clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__367.invoke(interruptible_eval.clj:129)
	at clojure.lang.AFn.run(AFn.java:24)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
	at java.lang.Thread.run(Thread.java:636)


 Comments   
Comment by Joe Snikeris [ 22/Jul/12 11:23 AM ]

I'm not sure which would be better: behave as if one wasn't passed in or return an error on the transport.

Comment by Chas Emerick [ 20/Aug/12 9:40 PM ]

Fixed to send an error with a status of "namespace-not-found".





[NREPL-11] Ensure efficient bytestring (byte[]) support in bencode transport Created: 21/Feb/12  Updated: 21/Aug/12  Resolved: 21/Aug/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0-beta9

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

In particular, no boxing of individual bytes or multiple en/decoding steps should be involved.

This implies that:

1. all string values should remain byte[] coming out of the bencode protocol support (which makes it more compliant with bencode anyway)
2. the string decoding should occur in the bencode transport impl
3. all string values should be decoded, except for those named in a reserved slot (say, "-unencoded")

All map keys should always be decoded to Strings.

While nREPL should continue to be fully compatible with Clojure 1.2.x, let's not kill ourselves trying to optimize for 1.2.x as well as Clojure >= 1.3.0; if necessary, just make sure the latter is fast. If 1.2.x benefits as well, then good.



 Comments   
Comment by Meikel Brandmeyer [ 22/Feb/12 2:05 PM ]

How about “nrepl/unencoded” re point 3?

Comment by Chas Emerick [ 22/Feb/12 5:57 PM ]

Is that intended to become a namespaced keyword at some point (further up in the stack than the bencode parser or transport, of course)?

If so, does that imply that other keys (perhaps "nonstandard" ones defined by middlewares) should be namespaced as well?

Also, this is actually impacting the protocol; nrepl/* makes me think the slot is related to an "application-level" handler provided by nREPL.

FWIW, my suggestion of -unencoded was a nod to Python's _foo and Clojure's -foo to denote private / internal members. *shrug*

Comment by Meikel Brandmeyer [ 23/Feb/12 5:03 AM ]

Namespaced keywords are indeed the inspiration. So that it should probably be more like clojure.tools.nrepl.transport/unencoded rather than just nrepl/unencoded. However it is not a requirement to be a real keyword. A qualified string works just as well. There could be a middleware which turns the map keys into keywords (considering also a possible namespace). ring has a similar middleware, IIRC. I think that qualified operation and result field values (in some form or the other) are a best practise in this respect.

However your argument of unencoded being part of the protocol is valid. But then why should it be private? Document it in the official protocol spec and let middlewares also add fields to it. The question is then: what happens with nested structures (eg. :status, which has to be traversed now painfully)? If middlewares weren't allowed to add to it, then only :value would be an interesting target for unencoded. So a simple flag would suffice.

In general, I still think a tagged protocol would have been the better solution since it also allows nesting and each slot knows whether it's encoded or not.

Maybe we should clarify what middlewares are allowed to do. Is there already some documentation on this? (I'm must confess I'm not up-to-date with all the design changes.)

Comment by Chas Emerick [ 01/Mar/12 3:32 PM ]

Re: encoding, I've started to think perhaps nothing should be decoded by default, and middlewares can independently handle such things. The standard middlewares will stake out one approach, but that at least leaves open the possibility of a different convention emerging over time that can be adopted later.

Opinions?

Comment by Chas Emerick [ 20/Aug/12 4:14 PM ]

Sorry, where is the code for this? No .patch here, and nothing in bitbucket looks right.

Leaving aside the code for now, defaulting to unencoded strings continues to seem reasonable; a default Transport impl/substrate can perform the en/decoding, as well as establish a default convention for indicating that some fields values should be left unencoded and should not be decoded.

Comment by Meikel Brandmeyer [ 20/Aug/12 4:21 PM ]

The code is here: https://github.com/kotarak/tools.nrepl/tree/bencode-improvements2

Comment by Chas Emerick [ 20/Aug/12 10:14 PM ]

Looks good, patch applied.

I'm less and less sure re: -unencoded. Thinking back to your "tagged protocol" comment, adopting a convention of keys like "value/binary" (or something similar) would be quite a bit more flexible and somewhat simpler to handle.

(Thinking of rich content, mime types could then be sent in e.g. "value/mime" — not that the transport would care about that, but it would at least be easy to pair up data with its "tag".)

Thoughts?

Comment by Chas Emerick [ 21/Aug/12 8:26 PM ]

Releasing -beta9 now; further discussion of unencoded value semantics can continue elsewhere…





[NREPL-20] Stray "#" in session-out is breaking (.println *out*) Created: 09/Jun/12  Updated: 09/Jun/12  Resolved: 09/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alex Osborne Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Patch: Code and Test
Waiting On: Chas Emerick

 Description   

There's a stray "#" in clojure.tools.nrepl.middleware.session/session-out which is turning the body of the (not off) case into a lambda that's never executed:

(cond
(number? x) (.append buf (char x))
(not off) #(.append buf x) ; <----
(instance? CharSequence x) (.append buf ^CharSequence x off len)
:else (.append buf ^chars x off len))

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

This is causing the newline character in (.println out) and (.println err) to be lost:

user=> (with-open [conn (repl/connect :port 7888)]
(-> (repl/client conn 1000)
(repl/message {:op :eval :code "(do (.println out \"1\") (.println out \"2\") (.println out \"3\")(flush))"})
doall
pprint))
({:out "123", ;; <--- where are my "\n"s?
...}, ...)

Removing the stray "#" fixes the problem:

{:out "1\n2\n3\n",
...}

Reported downstream by jaceklaskowski in https://github.com/technomancy/leiningen/issues/631

Patch with the trivial fix and unit test here: https://wok.meshy.org/~ato/0001-Remove-stray-hash-in-session-out-that-breaks-println-out.patch

Couldn't attach the patch to this issue as JIRA told me I don't have permission. I (Alex Osborne) have signed the Clojure CA and contribute this patch as per the terms of it.



 Comments   
Comment by Colin Jones [ 09/Jun/12 9:44 AM ]

Looks good, nice catch. Verified this test fails before the fix.

Chas, want me to apply/push this? I'm happy to do that, but wanted to wait since we haven't really talked about project workflow.

Comment by Chas Emerick [ 09/Jun/12 10:54 AM ]

Thank you very much — patch applied to master. I'll cut a new beta release on Monday so downstreams can pick it up.

Good eye, by the way, I'm sure I could have looked for days for that.





[NREPL-13] nrepl should support binding to a specific interface instead of all Created: 29/Feb/12  Updated: 29/Mar/12  Resolved: 29/Mar/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Enhancement Priority: Major
Reporter: Toby Crawley Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Patch: Code

 Description   

nrepl currently accepts a :port to bind to as an option, and uses that with the single-arg ServerSocket constructor, which binds to that port on all interfaces. I propose we add a :host option as well that can be used to specify the hostname or address of the interface to bind to.

Here is a potential implementation:

diff --git a/src/main/clojure/clojure/tools/nrepl/server.clj b/src/main/clojure/clojure/tools/nrepl/server.clj
index 9dbac40..b6f625b 100644
--- a/src/main/clojure/clojure/tools/nrepl/server.clj
+++ b/src/main/clojure/clojure/tools/nrepl/server.clj
@@ -8,7 +8,7 @@
                                             pr-values
                                             session))
   (:use [clojure.tools.nrepl.misc :only (returning response-for log)])
-  (:import (java.net Socket ServerSocket)))
+  (:import (java.net InetAddress Socket ServerSocket)))
 
 (defn unknown-op
   "Sends an :unknown-op :error for the given message."
@@ -64,7 +64,9 @@
 
 (defn start-server
   "Starts a socket-based nREPL server.  Configuration options include:
- 
+
+   * :host — the address or hostname of the interface that should be used;
+       defaults to all interfaces
    * :port — defaults to 0, which autoselects an open port on localhost
    * :handler — the nREPL message handler to use for each incoming connection;
        defaults to the result of (default-handler)
@@ -77,8 +79,10 @@
 
    Returns a handle to the server that is started, which may be stopped
    either via `stop-server`, (.close server), or automatically via `with-open`."
-  [& {:keys [port transport-fn handler ack-port greeting-fn] :or {port 0}}]
-  (let [ss (ServerSocket. port)
+  [& {:keys [host port transport-fn handler ack-port greeting-fn] :or {port 0}}]
+  (let [ss  (if host
+              (ServerSocket. port 0 (InetAddress/getByName host))
+              (ServerSocket. port))
         smap {:ss ss
               :transport (or transport-fn t/bencode)
               :greeting greeting-fn

I'm putting a CA in the mail today.



 Comments   
Comment by Chas Emerick [ 29/Mar/12 12:53 PM ]

Fixed on master, released as part of 0.2.0-beta3.





[NREPL-14] Exception when running -main from clojure.tools.nrepl.cmdline Created: 06/Mar/12  Updated: 20/Jun/12  Resolved: 20/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: 0.2.0

Type: Defect Priority: Major
Reporter: Trent Ogren Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Fix-calls-using-old-function-signatures.patch    
Patch: Code

 Description   

Running -main from clojure.tools.nrepl.cmdline gives the following exception:

Exception in thread "main" java.lang.IllegalArgumentException: No value supplied for key: 0
at clojure.lang.PersistentHashMap.createWithCheck(PersistentHashMap.java:89)
at clojure.core$hash_map.doInvoke(core.clj:355)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.core$apply.invoke(core.clj:600)
at clojure.tools.nrepl.server$start_server.doInvoke(server.clj:65)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.tools.nrepl.cmdline$_main.doInvoke(cmdline.clj:74)
at clojure.lang.RestFn.invoke(RestFn.java:397)
at clojure.lang.Var.invoke(Var.java:397)
at clojure.lang.AFn.applyToHelper(AFn.java:159)
at clojure.lang.Var.applyTo(Var.java:518)
at clojure.core$apply.invoke(core.clj:600)
at clojure.main$main_opt.invoke(main.clj:323)
at clojure.main$main.doInvoke(main.clj:426)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:405)
at clojure.lang.AFn.applyToHelper(AFn.java:163)
at clojure.lang.Var.applyTo(Var.java:518)
at clojure.main.main(main.java:37)

It looks like the repl/connect and start-server functions have changed since cmdline.clj was written. I attached a patch that updates their usage.



 Comments   
Comment by Chas Emerick [ 20/Jun/12 7:36 AM ]

Merged, thanks!





[NREPL-38] Certain Calendar values don't seem to be able to print Created: 12/Jan/13  Updated: 13/Jan/13  Resolved: 13/Jan/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: 0.2.1

Type: Defect Priority: Major
Reporter: Julian Birch Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug
Environment:

Ubuntu 12.10, running through Leiningen 2



 Description   

If I try to run lein repl outside of a project folder, I get Clojure 1.4.0.
I then run

(println (javax.xml.bind.DatatypeConverter/parseDateTime "2008-07-21T19:17:29"))

which produces

IndexOutOfBoundsException start 26, end 2, s.length() 28 java.lang.AbstractStringBuilder.append (AbstractStringBuilder.java:476)
#inst "2008-07-21T19:17:29.000+01:

Note the absence of a closing '"'.

My apologies in advance if this is impossible to reproduce.



 Comments   
Comment by Andy Fingerhut [ 13/Jan/13 9:55 AM ]

I can reproduce this with Clojure 1.4.0 and 1.5.0-RC2 if I do it within "lein2 repl" (I was using Leiningen version 2.0.0-preview10 to reproduce the problem).

If I use "java -cp clojure.jar clojure.main" to start a REPL session, with either Clojure 1.4.0 or 1.5.0-RC2 for clojure.jar, I don't see any problem. I was testing on Mac OS X 10.6.8 with Oracle/Apple JDK 1.6.0_37.

I also don't see this problem if I use Leiningen version 1.7.1, tested with both Clojure 1.4.0 and 1.5.0-RC2.

This appears to be some kind of bad interaction between Leiningen 2.0.0-preview10 and Clojure.

Comment by Andy Fingerhut [ 13/Jan/13 10:05 AM ]

I also reproduced this issue with the latest version of Leiningen, which is 2.0.0-RC2. Email sent to the Leiningen developer email list so they know about it.

Comment by Chas Emerick [ 13/Jan/13 10:52 AM ]

This is an nREPL bug involving an API mismatch between java.io.Writer.write() and java.lang.AbstractStringBuilder.append().

The fix is simple; patch release coming later today.

Comment by Chas Emerick [ 13/Jan/13 12:29 PM ]

Fixed with b9e930a1.

Will be a part of [org.clojure/tools.nrepl "0.2.1"], to be released later today.





[NREPL-16] nrepl.middleware.interruptible_eval/interruptible_eval raises a stack inconsistence Exception if the call to clojure.main/repl fails Created: 12/Apr/12  Updated: 20/Apr/12  Resolved: 16/Apr/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alexander Yakushev Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: bug
Environment:

clojure 1.4.0-beta5, Android



 Description   

If the call to clojure.main/repl in `evaluate' raises an exception before the :init part gets evaluated (so the expression `(push-thread-bindings @bindings)' is executed) then nREPL crashes with the following exception:

java.lang.IllegalStateException: Pop without matching push

It happens because after the underlining exception in clojure.main/repl is caught by the try block in `evaluate', the `finally' black calls (pop-thread-bindings) which were not actually "pushed".



 Comments   
Comment by Chas Emerick [ 16/Apr/12 5:34 PM ]

Fixed in 0.2.0-beta6. Please give it a try and see how it works on Android.

Comment by Alexander Yakushev [ 20/Apr/12 1:40 PM ]

It is OK now. OK in a sense that if something wrong happens inside the clojure.main/repl function then the stacktrace points there after the application dies.
Thanks for you help!





[NREPL-30] Investigate hangs when server goes away Created: 14/Sep/12  Updated: 08/Oct/12  Resolved: 08/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta9, 0.2.0-beta10
Fix Version/s: 0.2.0-beta10

Type: Task Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

See https://github.com/trptcolin/reply/issues/84#issuecomment-8481753 and maybe http://code.google.com/p/counterclockwise/issues/detail?id=405



 Comments   
Comment by Colin Jones [ 14/Sep/12 9:03 AM ]

If it helps, I did observe in some testing that after the server side of the socket goes down, writing to and flushing the client socket's output stream twice would bubble up a client exception. That seems a little keepalive-ish, but I'm not sure whether that sort of thing would do other transports any good.

Comment by Chas Emerick [ 03/Oct/12 12:37 PM ]

A fix for this is on master now, with commit 8a5dad2045434fcc06f2878de55f7dcdefa01a1b. There were a number of issues in the implementation of FnTransport that were preventing exceptions from bubbling up as they should have been.

No APIs were harmed in the course of implementing a fix. Also, no keepalive mechanism was required (which makes me slightly suspicious that the issue has actually been fixed, but we'll see). (Not TCP keepalive; that's irrelevant to the issue, and wouldn't help even if it were reliably standard/present.)

New tests added, and manual verification against nREPL servers running in different processes looks good too. I'll put out an 0.2.0-beta10 shortly so that people can start banging on this to see if it resolves issues in downstream projects/tools.

Note that I also found that it sometimes requires two sends (messages here, not writes to a socket) to provoke an error when writing to a transport that had been connected to a server that was closed or otherwise went away. Maybe it's a buffering issue? I don't have a good theory. Check out e.g. nrepl-test/transports-fail-on-disconnects if you want to play around with it.

Comment by Chas Emerick [ 04/Oct/12 5:06 PM ]

Note that the bencode transport should throw a java.net.SocketException when its connection is lost.

Comment by Chas Emerick [ 08/Oct/12 12:42 PM ]

Good reports received from downstream projects on the fix for this. Calling it good.





[NREPL-1] Whitespace is not printed via :out Created: 30/Sep/11  Updated: 21/Feb/12  Resolved: 21/Feb/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

This:

=> (println " ")
nil

Should be this:

=> (println " ")
 
nil

Perhaps :out isn't being sent if the content {{.trim}}s down to a zero-length string?



 Comments   
Comment by Chas Emerick [ 21/Feb/12 5:54 AM ]

Resolved in the redesign.





[NREPL-6] Add simple HOWTO to README for both tooling and application developers Created: 30/Sep/11  Updated: 21/Nov/12  Resolved: 21/Nov/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

...maybe push the protocol details into a /doc directory.






[NREPL-5] Add support for reading from stdin Created: 30/Sep/11  Updated: 21/Feb/12  Resolved: 21/Feb/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

Swank does this by polling back to the connected client to request input. In the face of asynchronous operations, this may be the only option.

Will require adding to the protocol.



 Comments   
Comment by Chas Emerick [ 21/Feb/12 5:58 AM ]

See clojure.tools.nrepl.middleware.session/add-stdin for the implementation. Userland documentation coming.





[NREPL-34] Default load-file middleware cannot load large files due to JVM method size limitations Created: 11/Oct/12  Updated: 11/Oct/12  Resolved: 11/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta10
Fix Version/s: 0.2.0-RC1

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

Simply interpolating contents of files to be loaded into a Compiler/load expression to be evaluated does not work with very large files. (See http://code.google.com/p/counterclockwise/issues/detail?id=429)



 Comments   
Comment by Chas Emerick [ 11/Oct/12 11:34 PM ]

Fixed in https://github.com/clojure/tools.nrepl/commit/77cb7d2347b0a78094ea32d7757aa3e3662fc634





[NREPL-52] Middleware linearizes wrong with redundant :expects entries Created: 02/May/14  Updated: 02/May/14  Resolved: 02/May/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Chas Emerick
Resolution: Duplicate Votes: 0
Labels: None


 Description   

Consider the following:

(def wrap-foo identity)
(mid/set-descriptor! #'wrap-foo
                     ;; note that these two entries are redundant since
                     ;; the session middleware implements "clone"
                     {:expects #{#'clojure.tools.nrepl.middleware.session/session "clone"}})

;; this one returns correctly
(mid/linearize-middleware-stack [#'wrap-foo
                                 #'clojure.tools.nrepl.middleware.session/session])
;; => (#'clojure.tools.nrepl.middleware.session/session
       #'user/wrap-foo)


;; this one returns incorrectly -- note that by adding a presumably
;; unrelated extra middleware to the stack, session and wrap-foo have
;; switched their order relative to each other.
(mid/linearize-middleware-stack [#'wrap-foo
                                 #'cider.nrepl.middleware.stacktrace/wrap-stacktrace
                                 #'clojure.tools.nrepl.middleware.session/session])
;; => (#'user/wrap-foo
       #'clojure.tools.nrepl.middleware.pr-values/pr-values
       #'cider.nrepl.middleware.stacktrace/wrap-stacktrace
       #'clojure.tools.nrepl.middleware.session/session)


 Comments   
Comment by Gary Fredericks [ 02/May/14 10:08 PM ]

I'm going to close this since I think NREPL-53 gets more to the root of the matter.





[NREPL-60] Tests failing in build "Unable to resolve var: clojure.pprint/use-method in this context" Created: 04/Aug/14  Updated: 19/Aug/14  Resolved: 19/Aug/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.4

Type: Defect Priority: Major
Reporter: Alf Kristian Støyle Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Moving-the-require-into-the-ns-declaration-makes-tes.patch    

 Description   

The tools.nrepl build has been failing since April 18th (http://build.clojure.org/job/tools.nrepl/269/console):

[INFO] Exception in thread "main" java.lang.RuntimeException: Unable to resolve var: clojure.pprint/use-method in this context, compiling:(clojure/tools/nrepl/server.clj:110:3)
[INFO] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6651)
[INFO] 	at clojure.lang.Compiler.analyze(Compiler.java:6445)
[INFO] 	at clojure.lang.Compiler.analyze(Compiler.java:6406)
[INFO] 	at clojure.lang.Compiler$InvokeExpr.parse(Compiler.java:3665)
[INFO] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6646)
[INFO] 	at clojure.lang.Compiler.analyze(Compiler.java:6445)
[INFO] 	at clojure.lang.Compiler.analyze(Compiler.java:6406)
[INFO] 	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5782)
[INFO] 	at clojure.lang.Compiler$TryExpr$Parser.parse(Compiler.java:2191)
[INFO] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6644)
[INFO] 	at clojure.lang.Compiler.analyze(Compiler.java:6445)
[INFO] 	at clojure.lang.Compiler.analyze(Compiler.java:6406)
[INFO] 	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5782)
[INFO] 	at clojure.lang.Compiler$FnMethod.parse(Compiler.java:5217)
[INFO] 	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3846)
[INFO] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6642)
[INFO] 	at clojure.lang.Compiler.analyze(Compiler.java:6445)
[INFO] 	at clojure.lang.Compiler.eval(Compiler.java:6700)
[INFO] 	at clojure.lang.Compiler.load(Compiler.java:7130)
[INFO] 	at clojure.lang.RT.loadResourceScript(RT.java:370)
[INFO] 	at clojure.lang.RT.loadResourceScript(RT.java:361)
[INFO] 	at clojure.lang.RT.load(RT.java:440)
[INFO] 	at clojure.lang.RT.load(RT.java:411)
[INFO] 	at clojure.core$load$fn__5066.invoke(core.clj:5641)
[INFO] 	at clojure.core$load.doInvoke(core.clj:5640)
[INFO] 	at clojure.lang.RestFn.invoke(RestFn.java:408)
[INFO] 	at clojure.core$load_one.invoke(core.clj:5446)
[INFO] 	at clojure.core$load_lib$fn__5015.invoke(core.clj:5486)
[INFO] 	at clojure.core$load_lib.doInvoke(core.clj:5485)
[INFO] 	at clojure.lang.RestFn.applyTo(RestFn.java:142)
[INFO] 	at clojure.core$apply.invoke(core.clj:626)
[INFO] 	at clojure.core$load_libs.doInvoke(core.clj:5528)
[INFO] 	at clojure.lang.RestFn.applyTo(RestFn.java:137)
[INFO] 	at clojure.core$apply.invoke(core.clj:626)
[INFO] 	at clojure.core$require.doInvoke(core.clj:5607)
[INFO] 	at clojure.lang.RestFn.invoke(RestFn.java:408)
[INFO] 	at clojure.tools.nrepl_test$eval5$loading__4958__auto____6.invoke(nrepl_test.clj:1)
[INFO] 	at clojure.tools.nrepl_test$eval5.invoke(nrepl_test.clj:1)
[INFO] 	at clojure.lang.Compiler.eval(Compiler.java:6703)
[INFO] 	at clojure.lang.Compiler.eval(Compiler.java:6692)
[INFO] 	at clojure.lang.Compiler.load(Compiler.java:7130)
[INFO] 	at clojure.lang.RT.loadResourceScript(RT.java:370)
[INFO] 	at clojure.lang.RT.loadResourceScript(RT.java:361)
[INFO] 	at clojure.lang.RT.load(RT.java:440)
[INFO] 	at clojure.lang.RT.load(RT.java:411)
[INFO] 	at clojure.core$load$fn__5066.invoke(core.clj:5641)
[INFO] 	at clojure.core$load.doInvoke(core.clj:5640)
[INFO] 	at clojure.lang.RestFn.invoke(RestFn.java:408)
[INFO] 	at clojure.core$load_one.invoke(core.clj:5446)
[INFO] 	at clojure.core$load_lib$fn__5015.invoke(core.clj:5486)
[INFO] 	at clojure.core$load_lib.doInvoke(core.clj:5485)
[INFO] 	at clojure.lang.RestFn.applyTo(RestFn.java:142)
[INFO] 	at clojure.core$apply.invoke(core.clj:626)
[INFO] 	at clojure.core$load_libs.doInvoke(core.clj:5524)
[INFO] 	at clojure.lang.RestFn.applyTo(RestFn.java:137)
[INFO] 	at clojure.core$apply.invoke(core.clj:626)
[INFO] 	at clojure.core$require.doInvoke(core.clj:5607)
[INFO] 	at clojure.lang.RestFn.invoke(RestFn.java:408)
[INFO] 	at user$eval1.invoke(run-test6023873211541663255.clj:1)
[INFO] 	at clojure.lang.Compiler.eval(Compiler.java:6703)
[INFO] 	at clojure.lang.Compiler.load(Compiler.java:7130)
[INFO] 	at clojure.lang.Compiler.loadFile(Compiler.java:7086)
[INFO] 	at clojure.main$load_script.invoke(main.clj:274)
[INFO] 	at clojure.main$script_opt.invoke(main.clj:336)
[INFO] 	at clojure.main$main.doInvoke(main.clj:420)
[INFO] 	at clojure.lang.RestFn.invoke(RestFn.java:408)
[INFO] 	at clojure.lang.Var.invoke(Var.java:379)
[INFO] 	at clojure.lang.AFn.applyToHelper(AFn.java:154)
[INFO] 	at clojure.lang.Var.applyTo(Var.java:700)
[INFO] 	at clojure.main.main(main.java:37)
[INFO] Caused by: java.lang.RuntimeException: Unable to resolve var: clojure.pprint/use-method in this context
[INFO] 	at clojure.lang.Util.runtimeException(Util.java:221)
[INFO] 	at clojure.lang.Compiler$TheVarExpr$Parser.parse(Compiler.java:659)
[INFO] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6644)
[INFO] 	... 69 more

This is for the version

  • clojure-1.3.0
  • clojure-1.4.0
  • clojure-1.5.0
  • clojure-1.6


 Comments   
Comment by Alf Kristian Støyle [ 04/Aug/14 1:13 PM ]

Seems the require functions is "not called correctly" when inside the try special form. Moving it to top level, or into the ns declaration resolves the issue. Attached a patch.

Comment by Chas Emerick [ 19/Aug/14 12:36 PM ]

Yeah, whoops. I think I did that thinking I was still trying to maintain Clojure 1.1 compatibility. Thanks for the patch, but I had already fixed it locally.





[NREPL-56] Test suite failing on JDK 1.8 Created: 23/May/14  Updated: 19/Aug/14  Resolved: 19/Aug/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.3
Fix Version/s: 0.2.4

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

e.g. http://build.clojure.org/job/tools.nrepl-test-matrix/154/CLOJURE_VERSION=1.6.0,jdk=Oracle%20JDK%201.8/console



 Comments   
Comment by Chas Emerick [ 19/Aug/14 12:38 PM ]

Thanks for the heads-up, fixed in 9c41e346053d6c3dbd7ae127b37b6582f894e9d2





[NREPL-58] Inconsistency between session and eval responses Created: 04/Jun/14  Updated: 19/Aug/14  Resolved: 19/Aug/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.4
Fix Version/s: 0.2.4

Type: Defect Priority: Major
Reporter: Jeff Valk Assignee: Chas Emerick
Resolution: Completed Votes: 1
Labels: bug, patch,

Attachments: Text File 0001-eval-session-consistency.patch    
Patch: Code

 Description   

At present, the interruptible-eval handler sends the :value or :eval-error status message before it updates the session to reflect the new binding result. This leaves the session values, particularly *e, *1, etc inconsistent with the state implied by those response messages.

As a practical example, if we want to wrap the "eval" op to check for an :eval-error and add error detail to the response from *e, (get @session #'*e) needs to hold the exception that resulted in the :eval-error. At present, however, it still holds the previous exception, leaving us no way to retrieve the current one in band.

The attached (minimal) patch makes ensures the session is updated before "eval" responses are sent.



 Comments   
Comment by Jeff Valk [ 04/Jun/14 1:00 PM ]

In the event that the attached patch seems a reasonable fix, I should probably add that my CA will be in the mail shortly.

Comment by Bozhidar Batsov [ 13/Jun/14 8:05 AM ]

I'm just here to pledge my support for Jeff's patch. Btw, Jeff, few days later and you could have signed the CA electronically.

Comment by Jeff Valk [ 13/Jun/14 9:32 AM ]

The irony did not escape me. Anyway, my CA (in dead tree form) is now on file.

Comment by Chas Emerick [ 19/Aug/14 1:56 PM ]

Looks good, thanks for the patch Applied as f02956d.





[NREPL-40] Thread leak in clojure.tools.nrepl.transport$fn_transport? Created: 11/Mar/13  Updated: 19/Aug/14  Resolved: 19/Aug/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.1, 0.2.2
Fix Version/s: 0.2.4

Type: Defect Priority: Major
Reporter: David Lao Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None
Environment:

Windows 7 x64, Oracle JDK 1.7.0.11 x64, clojure 1.4.0



 Description   

When trying out remote eval using your sample in the README, ie

(with-open [conn (repl/connect :port 59258)]
(-> (repl/client conn 1000) ; message receive timeout required
(repl/message {:op "eval" :code "(+ 2 3)"})
repl/response-values))

I'm noticing that hosting process leaking a thread each time the remote eval is called. Jconsole shows a clojure-agent-send-off-pool-xxx thread got spawn as result of the call. The stack appears to be pointing to the "while true" loop inside fn_transport.

java.util.concurrent.locks.LockSupport.park(LockSupport.java:186)
java.util.concurrent.SynchronousQueue$TransferStack.awaitFulfill(SynchronousQueue.java:458)
java.util.concurrent.SynchronousQueue$TransferStack.transfer(SynchronousQueue.java:359)
java.util.concurrent.SynchronousQueue.put(SynchronousQueue.java:878)
clojure.tools.nrepl.transport$fn_transport$fn__3912.invoke(transport.clj:44)
clojure.core$binding_conveyor_fn$fn__3989.invoke(core.clj:1819)
clojure.lang.AFn.call(AFn.java:18)
java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
java.util.concurrent.FutureTask.run(FutureTask.java:166)
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
java.lang.Thread.run(Thread.java:722)

What do you recommend as way to free the thread? I have server code that calls nrepl on behave of client connections and the number of call can pile up fairly quickly.



 Comments   
Comment by David Lao [ 12/Mar/13 3:37 PM ]

Here is my workaround.

— a/src/main/clojure/clojure/tools/nrepl/transport.clj
+++ b/src/main/clojure/clojure/tools/nrepl/transport.clj
@@ -36,12 +36,12 @@
to the 2 or 3 functions provided."
([read write] (fn-transport read write nil))
([read write close]

  • (let [read-queue (SynchronousQueue.)]
  • (future (try
  • (while true
  • (.put read-queue (read)))
  • (catch Throwable t
  • (.put read-queue t))))
    + (let [read-queue (SynchronousQueue.)
    + transport-thread (future (try
    + (while true
    + (.put read-queue (read)))
    + (catch Throwable t
    + (.put read-queue t))))]
    (FnTransport.
    (let [failure (atom nil)]
    #(if @failure
    @@ -51,7 +51,7 @@
    (do (reset! failure msg) (throw msg))
    msg))))
    write
  • close))))
    + (fn [] (close)(future-cancel transport-thread))))))
Comment by Chas Emerick [ 06/Aug/13 5:47 AM ]

Very nice catch. I'd love to have a patch from you; do you have a CA filed? (see http://clojure.org/contributing)

Comment by Chas Emerick [ 18/Apr/14 10:07 AM ]

Ping: did you send in a CA? Would love to apply a patch from you for this.

Comment by Chas Emerick [ 19/Aug/14 8:43 PM ]

I wanted to get this out as part of 0.2.4, so implemented my own patch, committed as 20a3d15. Thanks for the report!





[NREPL-65] Clients have no way to signal EOF when reading stdin Created: 24/Aug/14  Updated: 04/Sep/14  Resolved: 24/Aug/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.4
Fix Version/s: 0.2.5

Type: Defect Priority: Major
Reporter: Colin Jones Assignee: Colin Jones
Resolution: Completed Votes: 0
Labels: None


 Description   

As described in https://github.com/technomancy/leiningen/issues/1658

The client ought to be able to signal an EOF by sending a message like {:op :stdin :stdin nil}.



 Comments   
Comment by Colin Jones [ 24/Aug/14 11:00 PM ]

Closed by https://github.com/clojure/tools.nrepl/commit/eb526fd8498ced1b4bd1555f8ff680f3ad65f1b4

Comment by Chas Emerick [ 25/Aug/14 7:50 AM ]

Good, thanks!





[NREPL-66] *msg* is captured on first print Created: 10/Sep/14  Updated: 10/Sep/14  Resolved: 10/Sep/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.5
Fix Version/s: 0.2.6

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File fix_msg_capture.patch    

 Description   

The `msg` var is captured on first print by `interruptable-eval/evaluate`.

This means that all output is sent back to the client with the message id of the first message that prints.

See https://github.com/hugoduncan/tools.nrepl/commit/fcff5c8daa649a4c3996aa47d6132a3bb0ca77ce for a fix.



 Comments   
Comment by Hugo Duncan [ 10/Sep/14 8:47 AM ]

I think it affects 0.2.4 as well.

Comment by Hugo Duncan [ 10/Sep/14 8:53 AM ]

Patch from git commit above.

Comment by Chas Emerick [ 10/Sep/14 9:37 AM ]

Thanks!





[NREPL-7] Dependency on 1.3.0-master-SNAPSHOT instead of 1.3.0 Created: 15/Nov/11  Updated: 15/Nov/11  Resolved: 15/Nov/11

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Steve Lindsay Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

Running "mvn install" on tools.nrepl results in a build failure:

[INFO] Missing:
[INFO] ----------
[INFO] 1) org.clojure:clojure:jar:1.3.0-master-SNAPSHOT

Changing 1.3.0-master-SNAPSHOT to 1.3.0 in src/integration/clojure-1.3.0/pom.xml seems to fix it.



 Comments   
Comment by Chas Emerick [ 15/Nov/11 5:13 AM ]

Bizarre, since it is building cleanly here and on build.clojure.org.

I should certainly update the version number in that pom to 1.3.0 though. I'll do that shortly.

FWIW, maybe adding a -U to your mvn invocation would help? That forces a check of all snapshots.

Comment by Chas Emerick [ 15/Nov/11 5:22 AM ]

fixed on HEAD





[NREPL-39] Using functions reading from *in* causes "java.io.IOException: Write end dead" Created: 03/Feb/13  Updated: 07/Apr/13  Resolved: 21/Mar/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Adrian Bendel Assignee: Colin Jones
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Use-a-queue-for-stdin-to-avoid-PipedReader-Writer.patch    

 Description   

When calling (read-line) from lein repl or ccw repl, waiting around four minutes (on my machine),
evaluating any form causes the following exception:

ERROR: Unhandled REPL handler exception processing message {:code (+ 1 1), :id 08e30b70-ab0f-4a47-8937-3c040ce06f4f, :ns user, :op eval, :session 916200cc-e476-4ac4-98f6-76057a9020be}
java.io.IOException: Write end dead
	at java.io.PipedReader.ready(Unknown Source)
	at clojure.tools.nrepl.middleware.session.proxy$java.io.PipedReader$0.ready(Unknown Source)
	at java.io.BufferedReader.ready(Unknown Source)
	at java.io.FilterReader.ready(Unknown Source)
	at java.io.PushbackReader.ready(Unknown Source)
	at clojure.tools.nrepl.middleware.session$add_stdin$fn__1103.invoke(session.clj:197)
	at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__840.invoke(middleware.clj:17)
	at clojure.tools.nrepl.middleware.session$session$fn__1093.invoke(session.clj:165)
	at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__840.invoke(middleware.clj:17)
	at clojure.tools.nrepl.server$handle_STAR_.invoke(server.clj:18)
	at clojure.tools.nrepl.server$handle$fn__1132.invoke(server.clj:27)
	at clojure.core$binding_conveyor_fn$fn__4130.invoke(core.clj:1836)
	at clojure.lang.AFn.call(AFn.java:18)
	at java.util.concurrent.FutureTask$Sync.innerRun(Unknown Source)
	at java.util.concurrent.FutureTask.run(Unknown Source)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
	at java.lang.Thread.run(Unknown Source)

While googling a bit on this and without concrete knowledge of the implementation in nrepl,
it seems like closing a thread that wrote to PipedWriter, before closing the writer or writing to it from another thread,
causes PipedReader#ready to throw that exception.



 Comments   
Comment by Colin Jones [ 06/Mar/13 5:46 PM ]

I found this related article: http://techtavern.wordpress.com/2008/07/16/whats-this-ioexception-write-end-dead/

So it seems like we may need something a little more robust. I'm thinking just a LinkedBlockingQueue that gets written to on input, with a Reader proxy over the top to hook up to Clojure's LineNumberingPushbackReader. I've started fiddling with that idea, passing most of the tests already, and I think this is the right path to go down in general. Yell at me if you see a problem with this plan.

Comment by Chas Emerick [ 07/Mar/13 4:50 AM ]

I had no idea that PipedReader & co. actually track the threads that touch them. That makes them entirely inappropriate for the *in* case.

+1 to your plan, thanks for chasing this!

Comment by Colin Jones [ 08/Mar/13 3:28 PM ]

OK, this works for me against REPLy.

I'd have preferred to avoid the dynamic binding (skipping-eol), but I haven't been able to think of another way to accomplish what it does. The problem is that we don't want to send another :need-input message just to clear newlines, but we don't have any visibility into what's in the LineNumberingPushbackReader's buffer (neither the full deal, nor the pushback). My initial thought was to replace the LineNumberingPushbackReader directly, with a custom thing, but I think the assumptions around that class in `read` and `read-line` are too concretely baked in to allow it (especially before 1.5, when the buffer size became configurable).

And there's no room for adding bits to the Reader interface, so in order to effect a behavior difference in the Reader, depending on what the high-level state is, I don't know that there are any options aside from dynamic binding or an atom (and of the two, a var seemed more robust, no worries about finally & friends).

Comment by Colin Jones [ 21/Mar/13 11:39 PM ]

Pushed to master: https://github.com/clojure/tools.nrepl/commit/3cd3d0e8b2091fc70fde1587d3f0a995cbf8b283





[NREPL-21] Sometimes *err*/*out*/value messages are surprisingly ordered Created: 17/Jun/12  Updated: 20/Jun/12  Resolved: 20/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Flush-out-err-before-sending-value.patch    
Patch: Code

 Description   

As described here: https://github.com/technomancy/leiningen/issues/631, the *out*, *err*, and value printing are ordered differently than expected sometimes.

There are 2 things left here:
(1) The return value can sometimes print before synchronous prints to err/out. Synchronizing all the printing on the REPLy side doesn't seem to help. An example case:

=> (do (.print *err* "one") (print "two") (print "three"))
niltwothreeone

(2) Calling .println() on the *out*/*err* streams doesn't necessarily flush the stream. Changing this may have performance/network ramifications, but generally it seems like surprising behavior. An example case:

user=> (do (.println *out* "one") (.print *err* "two") (.print *err* "three") 1)
twothreeone
1

I'd expect two .print() calls to different streams to have undefined ordering, but the other orderings were all surprising.

The attached patch forces flushes to avoid both of these, for (1) immediately before sending the value message, and for (2) on the "normal" PrintStream [autoFlush] cases of println, printf, or format.

Side note: I couldn't get any tests to fail in this scenario, even after doing some reworking of the streams used in tests, but I kept one new test in this patch anyway. Turns out it's hard to write tests that really make sure .flush() gets called.



 Comments   
Comment by Chas Emerick [ 20/Jun/12 6:58 AM ]

Merged, thanks!





[NREPL-18] Flush *out*/*err* after N characters Created: 18/Apr/12  Updated: 20/Jun/12  Resolved: 20/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Flush-output-after-session-out-limit-characters.patch    
Patch: Code and Test

 Description   

Not sure how this should be configured, but it would be nice if doing something like (println large-seq-here) would flush output automatically after some number of characters. This is so that output could stream constantly across the nrepl connection instead of waiting for the entire output to send as one message (assuming no manual flushes).



 Comments   
Comment by Colin Jones [ 28/Apr/12 1:31 PM ]

There is a precedent for this, in the sense that many output buffers have some fixed size, and they are flushed when that size has been reached (e.g. http://www.gnu.org/software/libc/manual/html_node/Flushing-Buffers.html, http://docs.oracle.com/javase/6/docs/api/java/io/BufferedWriter.html)

We had talked about potential worries / headaches, where users may get partial lines printed at a time. But after more thought about it, I'm thinking we may be OK: concurrent out/err writes can already interleave in interesting ways, and depending on the placement of (flush) calls (explicitly or implicitly) to make them more atomic is something I'd already have expected to be fragile. Maybe others will see things I don't, though.

The implementation I have uses 1024 as the default, but that's pretty arbitrary. A less dynamic way of setting the limit would be also be fine with me, though since we're already in I/O-land, I'd assume performance isn't going to be bound by the dynamic var lookup.

Comment by Colin Jones [ 17/Jun/12 6:09 PM ]

Any thoughts on this one? It would be great to be able to print big seqs incrementally.

Comment by Chas Emerick [ 20/Jun/12 7:39 AM ]

Sorry for the delay, this fell through the cracks.

Looks good to me, except: can we make the "buffer size" an argument to the session middleware fn? That's where people will want to be able to customize it, if ever.

Commit and close this at will.

Comment by Colin Jones [ 20/Jun/12 11:14 PM ]

Done, thanks a bunch!





[NREPL-35] Eliminate several uses of reflection in tools.nrepl Created: 28/Oct/12  Updated: 31/Oct/12  Resolved: 31/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File nrepl-35-eliminate-reflection-v1.txt    

 Description   

There are several uses of reflection in tools.nrepl that can be eliminated with suitable type hints.



 Comments   
Comment by Andy Fingerhut [ 28/Oct/12 10:12 PM ]

nrepl-35-eliminate-reflection-v1.txt dated Oct 28 2012 adds type hints to eliminate some of the reflection warnings in tools.nrepl. There are still some left over that are not as obvious how to eliminate, if it is even possible.

Comment by Chas Emerick [ 31/Oct/12 4:44 AM ]

Thanks for the patch, but it overhinted in some areas (I try to keep hints at an absolute minimum to maximize readability) and overconstrained in others (e.g. ThreadPoolExecutor vs. Executor).

Fixed @ https://github.com/clojure/tools.nrepl/commit/ac95dc7da973f3d7edea75d0f6f9ce01ee18641d





[NREPL-37] Printing reference returned by clojure.tools.nrepl.server/start-server causes multimethod exception Created: 20/Dec/12  Updated: 26/Feb/13  Resolved: 26/Feb/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-RC1
Fix Version/s: 0.2.2

Type: Defect Priority: Minor
Reporter: Vaughn Dickson Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: bug


 Description   

I was accidentally printing the reference returned by start-server by calling it as the final function in my main method, which causes this error:

java.lang.IllegalArgumentException: Multiple methods in multimethod 'print-method' match dispatch value: class clojure.tools.nrepl.server.Server -> interface clojure.lang.IDeref and interface clojure.lang.IRecord, and neither is preferred
at clojure.lang.MultiFn.findAndCacheBestMethod(MultiFn.java:136)
at clojure.lang.MultiFn.getMethod(MultiFn.java:111)
at clojure.lang.MultiFn.getFn(MultiFn.java:119)
at clojure.lang.MultiFn.invoke(MultiFn.java:167)
at clojure.core$pr_on.invoke(core.clj:3266)
at clojure.core$pr.invoke(core.clj:3278)
at clojure.lang.AFn.applyToHelper(AFn.java:161)
at clojure.lang.RestFn.applyTo(RestFn.java:132)
at clojure.core$apply.invoke(core.clj:601)
at clojure.core$prn.doInvoke(core.clj:3311)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.main$eval_opt.invoke(main.clj:299)
at clojure.main$initialize.invoke(main.clj:316)
at clojure.main$null_opt.invoke(main.clj:349)
at clojure.main$main.doInvoke(main.clj:427)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:419)
at clojure.lang.AFn.applyToHelper(AFn.java:163)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.main.main(main.java:37)



 Comments   
Comment by Chas Emerick [ 02/Feb/13 7:35 AM ]

The fact that the returned record is also a clojure.lang.IDeref is a temporary compatibility measure, a result of changing to returning a record.

Adding a print-method implementation for the Server type that explicitly delegates to the IRecord implementation would resolve the problem.

Comment by Chas Emerick [ 26/Feb/13 4:42 AM ]

Fixed @ http://github.com/clojure/tools.nrepl/commit/0f016eb





[NREPL-22] CLONE - set stack size for Android Created: 30/Jun/12  Updated: 24/Jul/12  Resolved: 24/Jul/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.0.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Hank Assignee: Chas Emerick
Resolution: Duplicate Votes: 0
Labels: Android
Environment:

Android



 Description   

If you want to use nREPL on Android (currently only possible with sattvik's fork of Clojure) then you need bigger stack sizes than Android's default 8k to eval even moderately complex expressions. For comparison: A regular JVM on a PC has default stack sizes around 256k ... 512k depending on platform.

In nrepl.clj, modify

(doto (Thread. r)
(.setDaemon true))))))

to read

(doto (Thread. (.getThreadGroup (Thread/currentThread)) r "nREPL" 524288) ; 512k generous stack size
(.setDaemon true))))))

Note: There are warnings here http://bit.ly/u86tF1 about choosing a good stack size but since in practice nREPL seems to have <10 threads running with 1 active client connection we can be generous here.

The more advanced version would be to make this somehow configurable but that'd probably be over-engineering things.



 Comments   
Comment by Hank [ 30/Jun/12 9:21 AM ]

I understand that after a brief detour via agents we're now back to creating our own threads in nREPL. So the above applies gain. Apparently in JIRA you can't reopen issues so I'm 'cloning' this one.

Comment by Chas Emerick [ 24/Jul/12 4:52 AM ]

I was able to reopen NREPL-8; let's track it there.





[NREPL-41] Rebind print-.* dynamic vars when loading a file Created: 21/Mar/13  Updated: 21/Mar/13  Resolved: 21/Mar/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Colin Jones
Resolution: Completed Votes: 0
Labels: None


 Description   

Syntax errors happen when the user has bound things like *print-length*, *print-depth* and those take precedence.

See https://github.com/technomancy/leiningen/issues/952

Based on discussion in that ticket, I think the right approach is to rebind these vars when doing pr-str in load_file.clj - but I haven't yet been able to reproduce locally yet.



 Comments   
Comment by Colin Jones [ 21/Mar/13 11:38 PM ]

fixed on master: https://github.com/clojure/tools.nrepl/commit/11e3993746950640cb8173fbcd3b074c53b26ce0





[NREPL-9] (read-line) doesn't prompt for input after a (read) Created: 19/Feb/12  Updated: 21/Feb/12  Resolved: 21/Feb/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

When (read-line) is preceded by (read), it already has a newline in the stream, and returns "" immediately rather than prompting for input.

(read) then (read-line) works fine in the stock Clojure repl because the REPL shares STDIN with the in used by the normal (read) and (read-line) calls. So when the REPL's loop does clojure.main/repl-read (to get my evaluation input "(read-line)"), it skips whitespace before the read for the thing to eval, reads that thing, and then skips a newline, if that's the next thing: https://github.com/clojure/clojure/blob/master/src/clj/clojure/main.clj#L145-161

That allows the .read calls generated by the (read-line) execution to start after a trailing newline left in the stream.

We can do the same thing in nrepl.middleware.session/add-stdin on an eval message, but we need to check .ready first, to avoid sending unnecessary :need-input requests back to the client. I think this does belong in add-stdin, since it's essentially a stdin concern, even though it acts on the eval message.

I've got one test exposing the problem, and one proving we only eat newlines, not any old trailing character. I had to stick :stdin-reader on the session to enable consuming the newline from the middleware - not sure whether there's a way to do without it that I'm not seeing.



 Comments   
Comment by Chas Emerick [ 21/Feb/12 5:17 AM ]

Thanks for chasing this down! Patch applied to master.





[NREPL-10] SocketExceptions on transport/bencode when the other end has gone away Created: 19/Feb/12  Updated: 12/Oct/12  Resolved: 12/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0, 0.2.0-RC1

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

When running the tests, I'm occasionally getting SocketExceptions (which don't fail the test suite) with the message "Broken pipe". I think these stem from one end of the nREPL socket trying to write to the socket after the other end has gone away.

There's an existing test (ensure-closeable) that explicitly expects a SocketException to be raised if the socket has had .close called directly on it, rather than having the other end hang up, which I think is what the "Broken pipe" tells us. I'm curious as to whether this is important for some reason I haven't seen, or whether catching all SocketExceptions on transport/bencode's write function and returning nil would be appropriate.

If the end doing the socket closing actually should affect the behavior, we could sniff the exception message, but that feels a bit yucky.

I can actually reproduce this in "real life" by firing up an nREPL server / client (`reply --nrepl --port 9999`), then attaching to it (`reply --nrepl --attach 9999`), and in the attached client, run (Thread/sleep 10000) and immediately kill the process. Then the "Broken pipe" exception shows up in the original REPL with the server in the same process.

I've got a patch for this in my bitbucket fork (socket-exception branch): https://bitbucket.org/trptcolin/nrepl/changeset/4a432f3ce95a



 Comments   
Comment by Chas Emerick [ 21/Feb/12 5:35 AM ]

What do we really want to happen when a SocketException (or really, any error) occurs?

I'm all for better error handling/reporting, but it shouldn't obfuscate what's going wrong. Catching and squelching isn't doing anyone any favors. That could be particularly confusing if done at the transport level. "You mean all the messages I've been sending have been dropped on the floor silently, and I was supposed to be checking for a nil return?"

Comment by Colin Jones [ 26/Feb/12 12:46 PM ]

Yeah, you're right. Although, isn't checking for a nil return already necessary to account for timeouts?

What about having a server-side transport error handler that's rebindable, and re-throws by default? Or maybe we could collect errors somewhere, similar to what clojure.core/agent-error does.

The more I think about it, the less important this seems, though - mainly an annoyance when running tests.

Comment by Chas Emerick [ 12/Oct/12 12:51 AM ]

Was simply a disconnection stacktrace due to a client closing up prior to an evaluation response being written to the transport.

Fixed in https://github.com/clojure/tools.nrepl/commit/2d90bdd4fe14298bbfcaab52ecdea48781c71604





[NREPL-8] set stack size for Android Created: 30/Dec/11  Updated: 02/Feb/14  Resolved: 03/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.0.5
Fix Version/s: 0.2.0-beta9

Type: Enhancement Priority: Minor
Reporter: Hank Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: Android
Environment:

Android



 Description   

If you want to use nREPL on Android (currently only possible with sattvik's fork of Clojure) then you need bigger stack sizes than Android's default 8k to eval even moderately complex expressions. For comparison: A regular JVM on a PC has default stack sizes around 256k ... 512k depending on platform.

In nrepl.clj, modify

(doto (Thread. r)
(.setDaemon true))))))

to read

(doto (Thread. (.getThreadGroup (Thread/currentThread)) r "nREPL" 524288) ; 512k generous stack size
(.setDaemon true))))))

Note: There are warnings here http://bit.ly/u86tF1 about choosing a good stack size but since in practice nREPL seems to have <10 threads running with 1 active client connection we can be generous here.

The more advanced version would be to make this somehow configurable but that'd probably be over-engineering things.



 Comments   
Comment by Chas Emerick [ 30/Dec/11 9:14 AM ]

Noted, thank you. I'll try to make sure a provision for this gets into the next significant release.

My first inclination is to ensure that the stack size is configurable; a hardcoded size would likely be just as inappropriate in some environments as the system default.

Comment by Chas Emerick [ 21/Feb/12 5:21 AM ]

nREPL is now using Clojure's built-in agent threadpools. This should theoretically simplify things for you, correct? i.e. Isn't a patched build of Clojure necessary for use on Android for this stack size issue, among other reasons?

Comment by Hank [ 22/Feb/12 7:21 AM ]

The patched build is only necessary for the JVM byte code that the Clojure compiler dynamically generates to be converted to Dalvik bytecode. However adding additional android patches such as configuring the agent threadpool with a larger stack size do make sense. I'm not the official Android Clojure maintainer though – I guess there isn't an official one really – so my opinion on this only goes so far.

Comment by Chas Emerick [ 22/Feb/12 8:11 AM ]

Well, I think you have a solid case for expanding the scope of the android fork, or even for a change to parameterize the threadpools in Clojure itself.

I'm going to close this; there's really nothing to be done from within nREPL given that it doesn't maintain its own threadpool anymore.

Comment by Chas Emerick [ 24/Jul/12 4:49 AM ]

Reopening; since the use of agents was a fool's errand, parameterizing stack size needs to happen.

Comment by Chas Emerick [ 15/Aug/12 3:58 PM ]

Now that I look at this again, there are a number of options in nREPL to solve this.

First, you can provide an :executor argument to the interruptible-eval middleware. This could be any java.util.concurrent.Executor, so you can configure it to use whatever threads you want.

Second, and if you want to minimize work and retain as much of nREPL's default configuration otherwise, you can patch in your own thread pool by binding #'clojure.tools.nrepl.middleware.interruptible-eval/configure-thread-factory like so:

(def thread-group ...)
(def stack-size ...)

(defn android-thread-factory
  "Returns a new ThreadFactory suitable for use with android. This implementation
   generates daemon threads, with names that include the session id."
  []
  (let [session-thread-counter (AtomicLong. 0)]
    (reify ThreadFactory
      (newThread [_ runnable]
        (doto (Thread. thread-group
                runnable
                (format "nREPL-worker-%s" (.getAndIncrement session-thread-counter))
                stack-size)
          (.setDaemon true))))))

(with-bindings {#'clojure.tools.nrepl.middleware.interruptible-eval/configure-thread-factory
                android-thread-factory}
  (clojure.tools.nrepl.server/start-server ...))

The above totally untested, but I hope you get the idea. Give it a shot, and let me know how it goes. I'm going to take off this issue's fix target for now.

Comment by Chas Emerick [ 03/Oct/12 7:03 AM ]

I hear good things at this point about people connecting to nREPL endpoints running on Android. Please file another issue if this or other Android-specific problems arise in the future.

Comment by Jürgen Hötzel [ 23/Jan/14 7:01 AM ]

clojure.tools.nrepl.middleware.interruptible-eval/configure-thread-factory is a private non-dynamic var.

so i had to use with-redefs.

Are there any concerns making this dynamic?

Comment by Chas Emerick [ 23/Jan/14 8:31 AM ]

Not particularly, but with-redefs is a perfectly reasonable option (and likely safer, if you're really looking to patch in a completely different ThreadFactory.

What is prompting you to do so?

Comment by Jürgen Hötzel [ 31/Jan/14 4:08 PM ]

Yes. No need to use dynamic vars.

Comment by Alexander Yakushev [ 02/Feb/14 2:36 AM ]

OK, I finally integrated the fix into Neko. Thank you for help, Chas and Jürgen!





[NREPL-50] Configurable eval function Created: 10/Apr/14  Updated: 18/Apr/14  Resolved: 18/Apr/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.4

Type: Enhancement Priority: Minor
Reporter: Brandon Bloom Assignee: Chas Emerick
Resolution: Completed Votes: 1
Labels: patch, patch,

Attachments: Text File 0001-NREPL-50-Overridable-eval-function.patch    
Patch: Code

 Description   

#'clojure.main/repl has an :eval keyword option that is currently not provided by the interruptable-eval middleware.

I'd like to be able to supply that argument on a per session basis.



 Comments   
Comment by Brandon Bloom [ 10/Apr/14 7:36 PM ]

Attaching patch to support :eval option on (= op :eval) messages.

Comment by Chas Emerick [ 11/Apr/14 7:20 AM ]

This patch adds that option on a per-evaluation basis, which I don't think you really want. Having this set on a per-session basis (as you originally described) makes more sense to me, esp. insofar as doing it that way makes it trivially controllable from "userland" (i.e. the human typing in expressions to be evaluated somewhere). Changing the shape of messages going back and forth requires tool support.

Comment by Brandon Bloom [ 11/Apr/14 9:44 AM ]

Ah, sorry, I should have explained why I ultimately did this at the per-message level. I've got a custom evaluation, but I want to be able to freely mix standard and custom evaluations during the same session. In my case, I have .clj files that use clojure.core/eval and .eclj files that use eclj.core/eval via vim-fireplace. As far as I can tell, Fireplace only opens one session per lein project, so session variables would have been a lot more work.

Is there some way that I can easily override session variables on a per message basis? If so, then I can switch to that to get both the user-level customization and tooling-level support. Otherwise, I'm open to suggestions.

Comment by Chas Emerick [ 12/Apr/14 12:40 PM ]

Sessions are essentially just the result of get-thread-bindings, plus some nREPL bookkeeping. So, as long as evaluate binds e.g. *eval*, users can just set! the var to change it. Though, unless you watch for that in your custom evaluation fn, you'll never be able to change it back. :-P This seems like a good enough reason to keep the option per-message, and require tools/users to ask for what they want each time.

(Tangentially, I just remembered that Piggieback does roughly the same thing you're looking for. I thought about pushing something like what you're suggesting down into nREPL, but I'm very cautious about changing it to suit my needs [myopia and so on]. Glad we have a second real use case, which makes me feel better about the whole thing.)

Comment by Brandon Bloom [ 13/Apr/14 12:09 PM ]

Yeah, set! seems like a surefire way to steamroll over later evaluations unless everybody does a defensive set! all the time, and even then, set! may not even be defined for the evaluator that the caller is expecting!

Maybe this means that there needs to be some sort of general-purpose way to override session variables per message? Then, we'd only need to define eval and all messages could be processed inside their own with-bindings call.

Comment by Chas Emerick [ 14/Apr/14 2:13 PM ]

Maybe, but that's a lot to bite off without having more than "wouldn't it be cool if…" as motivation.

Any thoughts as to whether the eval symbol's namespace should be implicitly {{require}}d?

Will be applying the proposed patch (maybe with a tweak or two) this week.

Comment by Brandon Bloom [ 14/Apr/14 2:23 PM ]

Clojure's standard load-file looks for .class or .clj files. EClj has to patch the loader to look for .eclj files too, and eclj.core may be re-loaded after the new evaluator is bootstrapped.

I have to pre-load the namespace manually either way. Therefore, I don't care if it implicitly requires the namespace, as long as it doesn't :reload the namespace.

Comment by Chas Emerick [ 18/Apr/14 10:04 AM ]

Committed, will be available in 0.2.4-SNAPSHOT momentarily.





[NREPL-51] Pretty-printing reference returned by clojure.tools.nrepl.server/start-server causes multimethod exception Created: 12/Apr/14  Updated: 18/Apr/14  Resolved: 18/Apr/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.3
Fix Version/s: 0.2.4

Type: Defect Priority: Minor
Reporter: Michael Nygard Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: bug


 Description   

I was accidentally printing the reference returned by start-server by calling it as the final function in my main method, which causes this error:

java.lang.IllegalArgumentException: Multiple methods in multimethod 'print-method' match dispatch value: class clojure.tools.nrepl.server.Server -> interface clojure.lang.IDeref and interface clojure.lang.IRecord, and neither is preferred
at clojure.lang.MultiFn.findAndCacheBestMethod(MultiFn.java:136)
at clojure.lang.MultiFn.getMethod(MultiFn.java:111)
at clojure.lang.MultiFn.getFn(MultiFn.java:119)
at clojure.lang.MultiFn.invoke(MultiFn.java:167)
at clojure.core$pr_on.invoke(core.clj:3266)
at clojure.core$pr.invoke(core.clj:3278)
at clojure.lang.AFn.applyToHelper(AFn.java:161)
at clojure.lang.RestFn.applyTo(RestFn.java:132)
at clojure.core$apply.invoke(core.clj:601)
at clojure.core$prn.doInvoke(core.clj:3311)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.main$eval_opt.invoke(main.clj:299)
at clojure.main$initialize.invoke(main.clj:316)
at clojure.main$null_opt.invoke(main.clj:349)
at clojure.main$main.doInvoke(main.clj:427)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:419)
at clojure.lang.AFn.applyToHelper(AFn.java:163)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.main.main(main.java:37)



 Comments   
Comment by Michael Nygard [ 12/Apr/14 12:09 PM ]

I cloned this from NREPL-37, but cannot change any of the other fields. Thanks Jira.

Basically, this is the same dispatch problem as NREPL-37 but on clojure.pprint/simple-dispatch.

Version affected is 0.2.3.

Comment by Chas Emerick [ 12/Apr/14 12:21 PM ]

Thanks, should have thought of this when I fixed the related issue.

Comment by Chas Emerick [ 18/Apr/14 10:04 AM ]

Committed, will be available in 0.2.4-SNAPSHOT momentarily.





[NREPL-57] Add Java version info to op "describe"'s response Created: 29/May/14  Updated: 19/Aug/14  Resolved: 19/Aug/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.3
Fix Version/s: 0.2.4

Type: Enhancement Priority: Minor
Reporter: Bozhidar Batsov Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

Currently the describe op's reply features the relevant nREPL and Clojure versions. It'd be nice if the underlying JVM version was included as well.



 Comments   
Comment by Chas Emerick [ 19/Aug/14 1:45 PM ]

Added in cb7ce41





[NREPL-42] Reflection warnings in users' projects Created: 10/Jun/13  Updated: 19/Aug/14  Resolved: 06/Aug/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.2
Fix Version/s: 0.2.4

Type: Defect Priority: Minor
Reporter: John Hume Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: reflection

Attachments: Text File 0001-Eliminate-some-reflective-calls.patch    
Patch: Code

 Description   

In a project that uses clojure.tools.nrepl.server, we see a handful of reflection warnings from nrepl.

The attached patch eliminates those.



 Comments   
Comment by John Hume [ 11/Jun/13 9:14 AM ]

I should clarify that this is an issue in 0.2.3, but JIRA doesn't know about that version yet.

Comment by Chas Emerick [ 06/Aug/13 6:02 AM ]

Patch applied @ 1c53a172a8, thanks!

There is one remaining reflection warning, only because I culled the "fix" from your patch; the reflection in that case is helping us avoid the extra code IMO.





[NREPL-45] Laziness-forcing loses *out* bindings Created: 10/Dec/13  Updated: 19/Aug/14  Resolved: 25/Feb/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.2
Fix Version/s: 0.2.4

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 1
Labels: None

Attachments: File NREPL-45.diff     Text File nrepl-45.patch    

 Description   

This was reported in https://github.com/trptcolin/reply/issues/129

My current theory, outlined there, is that nREPL sets up session bindings for `out`, which since we're creating a lazy seq are lost before nREPL goes to print out the seq for transport back to the nREPL client (REPLy, vim-fireplace, etc.).

user=> (defn foo [] (println "a") (map (fn [x] (println "b") (throw (Exception. "oops"))) [1 2 3]))
#'user/foo
user=> (foo)
a

Exception oops  user/foo/fn--699 (NO_SOURCE_FILE:1)
user=> (doall (foo))
a
b

Exception oops  user/foo/fn--699 (NO_SOURCE_FILE:1)

Don't have time at the moment to dig in, but can do so later if nobody else gets to it first.



 Comments   
Comment by Colin Jones [ 21/Feb/14 5:19 PM ]

OK, more data: this is happening because of https://github.com/clojure/tools.nrepl/blob/master/src/main/clojure/clojure/tools/nrepl/middleware/pr_values.clj#L23, where the (with-out-str (pr v)) could be the first time a lazy sequence gets forced, so any side effects' printing gets thrown away.

The trick is distinguishing the actual side-effect printing from the printable value when the sequence is being forced. It seems like it comes down to this: we need to force lazy sequences in the same way that printing does, while keeping the value and the print output distinct. The attached patch does this (I opted to make a local instead of using the private #'clojure.core/pr-on).

I think this is good, but wouldn't mind a review before I push it.

Comment by Chas Emerick [ 25/Feb/14 4:28 AM ]

I'm going to reply on the reply ticket :-P, as I think the issue probably isn't nREPL.

As for the patch itself, I'm suspicious of printing values twice as a fix for anything. If output isn't being captured properly, then we should find the problem with how we're capturing output, not throw more output at the problem.

Comment by Chas Emerick [ 25/Feb/14 9:36 AM ]

Take a look at this, Colin. Your solution was correct, but can just drop into pr-values; with-out-str was too crude a hammer for what pr-values needs.

If you can, verify that this makes REPL-y (and any other tools you're interested in) work as expected. Feel free to commit it, or I can (you should have privs IIRC).

Comment by Colin Jones [ 25/Feb/14 11:56 AM ]

Yeah, I definitely like it better in pr-values, where the rest of the printing logic already lives. Committed as bbc5e02b665356e7dc33fe1d78346f17b3dec452.

Thanks!

Comment by Chas Emerick [ 25/Feb/14 12:04 PM ]

Nope, thank you. I'll get a release out sometime this week.





[NREPL-62] Improve Java version information Created: 22/Aug/14  Updated: 22/Aug/14  Resolved: 22/Aug/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.4
Fix Version/s: 0.2.5

Type: Enhancement Priority: Minor
Reporter: Bozhidar Batsov Assignee: Bozhidar Batsov
Resolution: Completed Votes: 0
Labels: enhancement

Attachments: Text File 0001-Extend-Java-version-information.patch     Text File 0001-Extend-Java-version-information.patch    
Patch: Code and Test

 Description   

As discussed before - structure the Java version information similarly to nREPL and Clojure's.



 Comments   
Comment by Chas Emerick [ 22/Aug/14 8:54 AM ]

My original hesitation was largely due to a suspicion that JVM version strings are not particularly regular, especially across vendors. Perhaps that simple regex will work on all Sun/Oracle VM's, but let's keep things clean in any cases where it won't: if no match is found, let's not include the major/minor/etc slots with nil values.

Comment by Bozhidar Batsov [ 22/Aug/14 9:32 AM ]

See the updated patch - if we don't find 4 numbers in the version string we don't include any extra slots.

Comment by Chas Emerick [ 22/Aug/14 1:41 PM ]

Thanks, applied. Looks like there's often no update version included, so I tweaked things so that 3-4 slots are acceptable. e.g. http://build.clojure.org/job/tools.nrepl-test-matrix/163/CLOJURE_VERSION=1.3.0,jdk=Oracle%20JDK%201.7/console (search for java.version).

Comment by Bozhidar Batsov [ 22/Aug/14 2:53 PM ]

Yeah, turns out the first build doesn't have an update. Thanks!





[NREPL-63] Add :version-string property to the Clojure version Created: 22/Aug/14  Updated: 22/Aug/14  Resolved: 22/Aug/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.4
Fix Version/s: 0.2.5

Type: Enhancement Priority: Minor
Reporter: Bozhidar Batsov Assignee: Bozhidar Batsov
Resolution: Completed Votes: 0
Labels: enhancement

Attachments: Text File 0001-Add-version-string-property-to-the-Clojure-version.patch    
Patch: Code and Test

 Description   

This aligns the Clojure version map with the nREPL and Java version maps (and simplifies the lives of tool writers).



 Comments   
Comment by Chas Emerick [ 22/Aug/14 9:24 PM ]

Applied as da0443d, thanks!





[NREPL-47] Two deftests with same name cause reduced test coverage Created: 23/Dec/13  Updated: 18/Apr/14  Resolved: 18/Apr/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.4

Type: Defect Priority: Trivial
Reporter: Andy Fingerhut Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: File nrepl-47-v1.diff    

 Description   

There are two deftest's with the name switch-ns in the same namespace. When this happens, the first deftest's tests are ignored, and never run. It is best if all tests are given unique names so this does not happen.

Found using a pre-release version of the Eastwood Clojure lint tool.



 Comments   
Comment by Andy Fingerhut [ 23/Dec/13 6:53 PM ]

Patch nrepl-47-v1.diff simply renames one of the deftest's named switch-ns to switch-ns-2.

Comment by Chas Emerick [ 18/Apr/14 10:06 AM ]

Thanks, fix applied.





[NREPL-61] Typo in README Created: 22/Aug/14  Updated: 22/Aug/14  Resolved: 22/Aug/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.4
Fix Version/s: 0.2.5

Type: Defect Priority: Trivial
Reporter: Bozhidar Batsov Assignee: Bozhidar Batsov
Resolution: Completed Votes: 0
Labels: documentation

Attachments: Text File 0001-Fix-a-typo-and-remove-some-trailing-whitespace.patch    
Patch: Code

 Description   

Just a small typo fix.



 Comments   
Comment by Chas Emerick [ 22/Aug/14 8:01 AM ]

Applied as 222c456. Thanks!





Generated at Sun Dec 21 12:00:33 CST 2014 using JIRA 4.4#649-r158309.