Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Side Effects in top level code => breaks use in compiled scenarios #533

Closed
jsa-aerial opened this issue May 29, 2018 · 7 comments
Closed
Labels

Comments

@jsa-aerial
Copy link

See how and when delayed-handlers is updated, and also that cider-nrepl-handler is built as a side-effecting top level form. This is the cause of (for example) #447. The key thing to note in the hack to work around this as presented in that issue is that cider.nrepl cannot be required at compile time (as in a ns form). So, until this is fixed, that should be explicitly mentioned (in bold!) in the documented workaround.

Expected behavior

Embedding an nREPL server in compiled situations (for example, uberjar application) by explicitly using / constructing middleware using cider.nrepl/cider-nrepl-handle supports connections with no problem

Actual behavior

Embedding an nREPL server in compiled situations (for example, uberjar application) by explicitly using / constructing middleware using cider.nrepl/cider-nrepl-handler results in NPE on any connection

Steps to reproduce the problem

  1. Embed an nREPL server by requiring cider.nrepl in an ns form
  2. Connect to the server using the Emacs CIDER client

Environment & Version information

cider-nrepl version

Any version of cider.nrepl since Sep 2017. The one I used was 0.17.0 The problem is how the deferred middleware was implemened

Java version

1.8

Operating system

Linux

@bbatsov
Copy link
Member

bbatsov commented Jun 16, 2018

Yeah, I guess we overlooked this initially. Hopefully @vspinu can take a look at this once he's done with the connection rework.

@vspinu
Copy link
Contributor

vspinu commented Jun 16, 2018

Unless I miss something this is an exact duplicate of #447 and #464.

@jsa-aerial
Copy link
Author

Sort of - Bozhidar explicitly asked me to make this issue (on Slack) as it explicitly notes that the problem is due to side effecting top level forms and where and how those effects are happening. Neither of the other two (I actually reference #447 in the description...) note the base problem or where it occurs.

If you want to get rid of this issue - go for it. Doesn't matter to me as long as it gets corrected

@vspinu
Copy link
Contributor

vspinu commented Jun 17, 2018

explicitly notes that the problem is due to side effecting top level forms and where and how those effects are happening.

I am afraid this is not that helpful (to me at least). By "side-effecting" you presumably mean updating the delays atom. Ok, and? What problems does that cause and through which mechanism more concretely? Do you have a proposal for an alternative implementation?

I will have a close look. It bit enough people that an intervention or an alternative middleware loader seems necessary.

@jsa-aerial
Copy link
Author

Absolutely - updating the delayed-handlers at compile time and running the code for cider-nrepl-handler at top level is guaranteed to not work in any AOT (uberjar with main for example) situation. That's due to the fact the namespace must be loaded in order to compile the one requiring it. That will always invoke all top level side effects. So, you have to change all of that so that nothing happens until runtime. Best way is to remove the updates from the macro(s) and have an initialization function which is used at startup to initialize things.

Have a look at the answer to this as it might help you:

https://stackoverflow.com/questions/32288195/why-lein-uberjar-evaluates-variables-defined-with-def

If none of this helps, I really don't know what else to say...

@bbatsov bbatsov added the bug label May 6, 2019
@arohner
Copy link

arohner commented Aug 22, 2019

I just encountered this as well.

One way of rephrasing the problem is to think in terms of when macros run. Macros run at compile-time, and generate code that is used at runtime. At a conventional repl, those occur right after each other. When doing AOT, compile time happens when the classfiles are generated, but runtime might happen on a different machine entirely.

So the problem isn't "side effects at the top level", it's "side-effects at the top level in macros while using AOT"

@bbatsov
Copy link
Member

bbatsov commented Nov 24, 2020

This should be fixed now - see #684.

@bbatsov bbatsov closed this as completed Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants