[Bug]: Non-daemon executor thread in Clojure prevents clean JVM shutdown

[oliyh]

What happened?

Hi,

Firstly thank you for the great library, it transformed the way I thought about building UIs and it's so much more elegant than redux and so much cleaner than React hooks.

I have a library called re-graph which uses re-frame mostly for state management of websocket lifecycle. It runs in both cljs and clj, and we have uncovered an issue with the clj implementation which is preventing clean shutdown of the JVM because of an executor thread which is left running.

The full conversation is here: oliyh/re-graph#75

@lowecg has produced some code which works around the issue, but I think this would be best incorporated into re-frame (or maybe re-frame-test) and I'd be grateful to know what you think.

Thanks for your time

Version

1.2.0

What runtimes are you seeing the problem on?

JVM (CLJC)

Relevant console output

No response

[kimo-k]

Hey, I'd be happy to look into this. However, day8 are in crunch mode, so we can't spare much time until next Spring.

Please make a PR with explanation & references to the java features. The more straightforward the writing, the sooner we can merge it.

[lowecg]

I've created a minimal project demonstrating the issue: https://github.com/lowecg/re-frame-issue-809

I'll issue a PR shortly

[lowecg]

@kimo-k / @oliyh Thanks for you time on this. PR raised - I hope it meets expectation :-) Let me know if you need anything else.

[kimo-k]

My takeaways so far:

• calling dispatch actually pushes an event (vector) onto a queue
• when re-frame pulls an event from the queue (to call the event handler), that's called a tick
• this queue defers to the host environment, leaving time for stuff like draw calls & (non-re-frame) event loops
• when running re-frame on the JVM, ticks are handled by an executor.
• the executor needs an explicit shutdown call. Otherwise, if you try to exit the JVM process, it fails to exit.
• some people want to reuse the executor. What exactly does that mean?

[lowecg]

Thanks @kimo-k, that's accurate.

some people want to oliyh/re-graph#75 (comment). What exactly does that mean?

If you use AWS Lambda with the JVM, then you want to likely want to use SnapStart to cut down cold start times. SnapStart requires you to prime your app by touching necessary classes in a warm up phase. This hits the executor.

I've just been doing some testing without interfering with the executor and it's working ok. So let's leave out the reuse after shutdown

[kimo-k]

The PR where we added the executor has the premise that building a full app in .clj is a non-goal for re-frame.

I suppose that premise still holds - we have no ambitions to fully support .clj apps.

That seems to rule out the need to change re-frame so that re-graph can work. Maybe it's better if re-graph can just use the workaround.

However if this problem affects re-frame-test, that does seem more serious.

[kimo-k]

@lowecg, @oliyh, in your opinion, would it suffice to remove the executor altogether? Instead, next-tick would simply be (fn next-tick [f] (f) nil). We'd no longer call event handlers on a separate thread.

Looking at the context so far, it seems like the benefits of the executor could be more cosmetic than pragmatic. And we have observed at least one drawback to the executor (#469, #471).

[oliyh]

Hi @kimo-k thanks for the time you have spent on this. I'm agnostic about how next-tick is implemented, I consider that re-frame internals and I don't know of anything that depends on this.

[lowecg]

@kimo-k - I agree with @oliyh. I'm happy to use the workaround in the meantime.