Updated runMultipleEvents to fix Issue #33

[fluffynukeit]

This request includes some extraneous changes. The important ones for remedying issue #33 are the changes to the runMultipleEvents function from CPS to iterative. This doesn't completely fix the possibility of hitting the recursion limit since the rest of the driver code is in CPS, but it should help. It resolved my issue and I'm putting up many thousands of elements in my GUI.

[HeinrichApfelmus]

Thanks!

Unfortunately, fixing this issue is not as simple as replacing the recursive call with a for loop. While most events are just tail calls indeed, some of them make an ajax request via the function signal. Processing events may not continue before this request has received a response.

So, the underlying issue here is that in order to avoid hitting the recursion limit, we need to make synchronous ajax calls without passing continuations.

---

Personally, I would avoid investing much effort into the current backend code. "The Right Way" to fix this is to use WebSockets for the client/server communication instead. We want to reduce latency anyway.

[fluffynukeit]

I'm not sure I follow. When you say "may not continue," do you mean that it must not continue, cannot continue, or might not continue?

I am investing some small amount of time in the current driver code because I am still working on the old dev version of TPG, and I don't want my progress in FNIStash to be impeded while waiting for updates to the HEAD version that I'm not using yet. So I want to ensure I understand any problems with my "quick fix" solution so they don't bite me in the short term. I haven't encountered a problem yet but I admit I haven't tested the fix exhaustively.

[HeinrichApfelmus]

I'm not sure I follow. When you say "may not continue," do you mean that it must not continue, cannot continue, or might not continue?

Beats me. Looking at the source codes for the router and signal function in the module Graphics.UI.Threepenny.Internal.Core, it seems that the client does not need a response from the server to proceed with event processing. I don't see an opportunity for race conditions either, because the signals are written into a channel on the server side. Your fix is probably fine, though I'm hesitant to accept the pull request.

[HeinrichApfelmus]

I'm currently looking into the internals and it occurs to me that a potential problem is the following: when the client makes two asynchronous AJAX calls with the signal function without using continuations to order them, there is no guarantee that the server processes the HTTP requests in the order that the client made them. Not very likely, granted, but since the client and server may run multiple threads, there is potential for a race condition. This is not an ordered communication over TCP, after all.

Likewise, an event handler on the client might make a request before it finishes making a signal request. For this case, Chris Done's implementation actually contains provisions (using sMutex and duplicating the channel in call), I doubt he would have put them in if there was not potential for a problem. Am I understanding this correctly @chrisdone ?

[HeinrichApfelmus]

@daniel-r-austin I think I know what's going on:

• When the runEvent function wants to reply to the server, we need to use the signal function with a continuation in order to make sure that the values are sent in the correct order. (Keep in mind that asynchronous requests may be executed in parallel, we have no guarantee that the server processes them in the order that the JS code initiated them.) Hence, your fix would be incorrect.
• Very often, however, the runEvent function does not need to reply to the server and simply calls the continuation. This is were the recursion stack comes in. In the previous case, the continuation was attached to an event handler to be called later, whereas in this case, we make a direct call ourselves.

A correct fix would use the continuation in the first case, but avoid a recursive call in the continuation in the second case. This can be done by supplying a clever reply function to runEvent in my newest refactorings https://github.com/HeinrichApfelmus/threepenny-gui/commit/6933ba48da54316f15bde634a2ae55f31fd63c5e , though at this point, it's probably simpler to just switch to the new WebSockets implementation.

[HeinrichApfelmus]

This seems to be obsolete now that I've implemented WebSocket support.