reflex-frp / reflex-vty

Build terminal applications using functional reactive programming (FRP) with Reflex FRP.
https://reflex-frp.org
BSD 3-Clause "New" or "Revised" License
140 stars 26 forks source link

An apparent race condition in `mainWidget`'s shutdown event handling #84

Open deepfire opened 2 months ago

deepfire commented 2 months ago

This one does terminate, as expected:

mainWidget $ getPostBuild >>= delay 1

..but neither of the following do:

mainWidget getPostBuild
mainWidget now
deepfire commented 2 months ago

Just in case -- the following also terminates.

mainWidget $ getPostBuild >>= delay 0
maralorn commented 2 months ago

Comparing

https://github.com/reflex-frp/reflex-vty/blob/develop/src/Reflex/Vty/Host.hs#L157

and

https://github.com/reflex-frp/reflex-vty/blob/develop/src/Reflex/Vty/Host.hs#L195

It seems you are right. The postBuild event is being handled before the normal event loop started and that firing does not handle the shutdown event. I don’t know if this should be considered bug. An application which shuts down on postBuild seems a bit useless. :laughing:

deepfire commented 2 months ago

@maralorn, what if semantics of the application are to perform conditional immediate termination?

I've not looked into the details of how large the race window is -- what amount of computation can be practically done within it -- as well as how dependent is the size of the race window on the parallelism/single-thread performance of the system and the OS/RTS thread scheduling behavior.

That said, I do agree it's a rather niche issue.

maralorn commented 2 months ago

Yeah, I admit I had thought about the conditional thing, too. It might make sense in some situations.

This is not actually a race condition in the sense of a timing issue. Reflex event propagation is actually pretty deterministic and not concurrent. You could in theory delay this postbuild event with an arbitrarily computationally slow function. Or e.g. with a threadDelay in the executed function. (But not with something using performEvent or the reflex delay function). You would still encounter the race condition. Because handling of the shutdown event will only start after the postbuild event has completely been processed. At least that is how I understand the code …