adzerk-oss / boot-reload

Boot task providing live-reload of browser css, images, etc.
Eclipse Public License 1.0
87 stars 35 forks source link

Shutdown httpkit server on task cleanup #107

Closed bhagany closed 7 years ago

bhagany commented 7 years ago

When running boot-reload from a repl, successive runs leave running servers in the background, which means increasing resource usage, and potential hard-to-diagnose port conflicts.

martinklepsch commented 7 years ago

Looks good to me 👍

One thing to be careful about is that the task may be invoked multiple times in which case the stop-fn atom should not get reset but since this is all ran in a pod (per reload invocation) it seems not to be an issue.

Deraen commented 7 years ago

Good catch.

I guess people just haven't noticed this thanks to the random ports.

vikeri commented 7 years ago

Could this issue possibly also exist in boot-cljs-repl when one specifies :nrepl-opts {:port 1339}? I namely think I just ran into that.

Deraen commented 7 years ago

@vikeri Not exactly. Boot-cljs-repl starts the Browser REPL WebSocket server using Weasel and closes that on cleanup-hook. But boot-cljs-repl also uses built-in REPL task which starts the nREPL server, and I don't think that is closed: https://github.com/boot-clj/boot/blob/master/boot/core/src/boot/task/built_in.clj#L477