lutzer / node-red-contrib-finite-statemachine

A finite state machine implementation for node red.
MIT License
16 stars 5 forks source link

In a loop, the trigger object grows recursively #23

Closed OriolFM closed 2 years ago

OriolFM commented 2 years ago

I was using the finite-statemachine in a project to control several mobile robots.

I often had the problem that the CPU/memory usage in node-red grew up quickly, and either some of the loops were interrupted after some time (my guess is they reached max size for a node-red message) or the whole node-red application crashed (I guess several concurrent messages had grown too much without reaching the maximum size, but node-red couldn't handle all of them at once).

After some inspection over the full msg, I realised that msg.trigger included all of the previous message (not just the previous payload), so on each iteration, trigger includes all the previous message, including the trigger. While this is certainly useful if you're aware of it, on the documentation just states that payload.trigger "Contains the original message that triggerd the state change."

First of all, msg.payload.trigger does not exist. the trigger message is stored in msg.trigger instead (thus I couldn't figure out what the problem was until I looked at everything).

Second, the node does not clear msg.trigger of the trigger message before replicating it and assigning it to the new msg. trigger, so in a loop, the new msg will contain the previous msg in msg.trigger, that will contain the previous msg in msg.trigger, and so on, recursively, until node-red runs out of memory and crashes, or until the msg breaks the max msg size and stops.

How did I fix it in my application? just clear msg.trigger in a function node or a change node before feeding it back into finite-statemachine node:

msg.trigger = {};

Now, since it might be a useful tool for debugging, I think at least the documentation should be changed to warn users of this (finite state machines are often used in loops) or, at best, include an option within the node to automatically clear msg.trigger on input (for normal operation), or disable it (for debugging, with the warning that the message will grow and can crash the application).

Thanks.

lutzer commented 2 years ago

Thank you @OriolFM. You are absolutely right. That was a major bug in the current statemachine version. I fixed it in v2.1.0. The statemachines output will now only contain the last trigger message. At the same time i found a major bug in the data handling with recursion. it is now also fixed, but i had to introduce some breaking changes for the data handling. see the manual or examples for details.