search

matthewp / robot

🤖 A functional, immutable Finite State Machine library
https://thisrobot.life
BSD 2-Clause "Simplified" License
1.92k stars 88 forks source link

**CRITICAL BUG** Interpret reference #185

Open kybarg opened 1 year ago

kybarg commented 1 year ago

Issue

Seems like interpret returns only a referense to origincal service. So after trying to remove service or referense, service keeps running.

Reproduction

https://codesandbox.io/s/awesome-thunder-rk87mv?file=/src/index.js

matthewp commented 1 year ago

I don't think I understand the bug. You send 2 events to the service which puts it into the child machine. You then create a new service. That doesn't stop the in-flight setTimeout from happening.

Creating a new service doesn't "stop" the existing service. There isn't presently a way to stop a service that's already running, aside from not sending events into it any more.

matthewp commented 1 year ago

A service.stop() would be a good feature to add though.

kybarg commented 1 year ago

@matthewp any suggestions how this can be achieved? And also don't you find this wrong that reference can be removed and you loose control over state machine, but service is still running?

matthewp commented 1 year ago

Add a stop() function here: https://github.com/matthewp/robot/blob/main/machine.js#L189 that probably just sets a this.stopped = true. Then probably send() checks if the service is stopped at the top of the function and exits if so: https://github.com/matthewp/robot/blob/main/machine.js#L169

No, I don't think this is wrong, that's just how JavaScript works. If you don't keep a reference to an object that does stuff... it's going to keep doing stuff.

I do think your example is not something you'd want to happen! That's why I suggested a stop().

However, even with a stop() it's not going to prevent the Promise from resolving. So if there are any mutations inside of the setTimeout handler those are still going to happen. Nothing we can do about that. But we can avoid the machine from continuing to change.

kybarg commented 1 year ago

@matthewp maybe you are right. Im my appp I use state machine as session manager. So sometime people leave in the middle of the session and it is destoyed (at least I thought it was the case) after timeout. Now it is a memory leak in my case😂 Another option was to implement "terninate" transition for each state, but my app is too big to do so.