Skip to content

Conversation

@IvaTutis
Copy link

No description provided.

@IvaTutis IvaTutis changed the title Mutex Added the option of firing triggers in a threadsafe manner on the state machine, enabling usage of SM as a shared resource between threads Sep 13, 2024
@IvaTutis
Copy link
Author

This works brilliantly when using SM as eg. a singleton in a multithreaded app. I'm eager to hear your feedback.

@leeoades
Copy link
Contributor

leeoades commented Dec 2, 2024

Hi @IvaTutis,
I can't see any unit tests. It would be well worth creating some that bombard the state machine with multiple threads in order to prove integrity.

@leeoades
Copy link
Contributor

leeoades commented Dec 2, 2024

That said, I would not personally recommend this approach. Instead, I would wrap the state machine within a component that handles the thread-safety. There is no need to introduce such complexity into the state machine itself.

@crozone
Copy link
Collaborator

crozone commented Dec 3, 2024

Please see discussion #527

We have not settled on a strategy, but we are more likely to eventually use a dedicated firing mode (FiringMode.Serial) that internally serializes triggers, rather than using dedicated method overloads.

@felixcollins
Copy link

If you are not introducing thread safe triggers (or if you do; when the machine is not in FiringMode.Serial) there should be a possibility to globally intercept all Fire calls. This could then be loaded with an Assert to enforce calling thread == instantiating thread. Or alternatively (in a non-concurrent but multi-threaded system) an Assert that a semaphore or other synchronisation primitive is in the correct state.

Another way of describing this, is to say there should be an aspect oriented provision to extend the Fire methods - similar to the OnTransitioned Event. Perhaps event BeforeFiring?

Other than forking and implementing this myself, is there any way to add this sort of protection currently? I think simply having a Debug-time check that the library is being used non-concurrently, as designed, would go a long way to reducing the support issues. Should I add separate enhancement request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants