weavejester / integrant-repl

Reloaded workflow functions for Integrant
MIT License
158 stars 17 forks source link

Prevent simultaneous `(go)` from colliding #13

Closed SevereOverfl0w closed 5 years ago

SevereOverfl0w commented 5 years ago

Currently, if you type (go) twice (e.g. your emacs is configured to auto-run it, and you also type it while it is starting) then you can get into a bad state where you have a system running that you no longer have a reference to.

One solution to this is using a (lock) around alter-var-root modifications to system/config/etc. This might be a sign that an agent is more appropriate in order to serialize modifications to that var.

Edge contains such automation out of the box for emacs users. (cljs-repl) is run in the pending-cljs repl automatically, (cljs-repl) will run (go) , and users are also typing (go) in the clj repl.

I can fix this common case in Edge directly, but I thought that a solution may make sense at this level. In Edge I would have to wrap all of the repl functions.

weavejester commented 5 years ago

Adding some thread safety seems fine. Would a simple locking block on the var work?

SevereOverfl0w commented 5 years ago

I think locking around where the alter is done should work, yeah.

SevereOverfl0w commented 5 years ago

Having tried a lot to reproduce this without figwheel, I can't! I think things must be working as they are. My attempts were doing things like:

(dotimes [_ 30] (future (go)))

I believe I was actually encountering that figwheel-main will start a server in the background if you run cljs-repl and one isn't running (reported at https://github.com/bhauman/figwheel-main/issues/183). Might be relevant to duct.

weavejester commented 5 years ago

Thank you for the investigation.