pytransitions / transitions

A lightweight, object-oriented finite state machine implementation in Python with many extensions
MIT License
5.52k stars 525 forks source link

Thread safe machine without locking #388

Closed mla-martin closed 4 years ago

mla-martin commented 4 years ago

Hi,

I would like to implement a transition variant that can be used with threads but does not block until a thread is done. See the simple example below. The calls in the run methods are never executed because a lock is held (due to the thread created/started) in the constructor. Actually I read the source of the LockedMachine but did not understand how the trigger function actually has knowledge of the additional thread I'd created. However - when not using the LockedMachine but the plain Machine variant the example works as expected (e.g. B->C->A->B->C->A->A->B->C) but I think it is not thread safe since the trigger method is not reentrant safe. Is there is an implementation with a reentrant safe trigger but no locking of additional threads?

import time
from transitions.extensions import LockedMachine as Machine
#from transitions import Machine #This works as expected
import threading

class Test(Machine):
    states=['A','B','C']
    def __init__(self):
        Machine.__init__(self, states=self.states,
                                initial='A'
                                )
        self.thread=threading.Thread(target=self.thread_run)
        self.thread.start()

    def thread_run(self):
        while True:
            time.sleep(7)
            self.to_A()

    def run(self):
        while True:
            self.to_B()
            self.to_C()
            self.to_A()

    def on_enter_A(self):
        print("->A")
        time.sleep(1)
    def on_enter_B(self):
        print("->B")
        time.sleep(1)
    def on_enter_C(self):
        print("->C")
        time.sleep(1)

t=Test()
t.run()        

`
import time
from transitions.extensions import LockedMachine as Machine
#from transitions import Machine #This works as expected
import threading

class Test(Machine):
    states=['A','B','C']
    def __init__(self):
        Machine.__init__(self, states=self.states,
                                initial='A'
                                )
        self.thread=threading.Thread(target=self.thread_run)
        self.thread.start()

    def thread_run(self):
        while True:
            time.sleep(7)
            self.to_A()

    def run(self):
        while True:
            self.to_B()
            self.to_C()
            self.to_A()

    def on_enter_A(self):
        print("->A")
        time.sleep(1)
    def on_enter_B(self):
        print("->B")
        time.sleep(1)
    def on_enter_C(self):
        print("->C")
        time.sleep(1)

t=Test()
t.run()        
aleneum commented 4 years ago

Hi @martin-intex,

this looks like a question about how to use transitions. Please refer to Stackoverflow for such matters. Posting there has several advantages:

Your question gains higher visibility since most developers look for help there The targeted community is larger; Some people will even help you to formulate a good question People get 'rewarded' with 'reputation' to help you. You also gain reputation in case this questions pops up more frequently. It's a win-win situation.

About the first part of your question:

Actually I read the source of the LockedMachine but did not understand how the trigger function actually has knowledge of the additional thread I'd created.

it does not. Every execution of a machine method will acquire the lock and only release it when it returns. By default, LockedMachine uses simple locks (threading.Lock) which can only be acquired once. Calling a trigger from withing a trigger (or any other machine method) will cause a deadlock. Have a look at threading.RLock and the relevant transition documentation how to use LockedMachine with custom contexts.

Currently, this issue neither contains a feature request nor a bug report. I will close it for now. Feel free to comment nevertheless. If a feature request or a bug report arises, I will reopen the issue again. Additionally, if you post this question on SO, you can add the link here and notify me if the question stays unanswered for quite some time.

mla-martin commented 4 years ago

Hi,

it does not. Every execution of a machine method will acquire the lock and only release it when it returns. By default, |LockedMachine| uses simple locks (|threading.Lock|) which can only be acquired once.

That is what I had expected and what I also relied on. But unfortunately this is not what I see.

Based on the "lock only during the function call" I assume that the main loop in the method will not be deadlocked since the actual call

in the thread will be done quite fast and shall release the lock. There is no need to held the lock during the 7 second sleep. So I think there is either some unwanted side effect with that construction or the documentation for that behaviour is misleading. Since this simple example does not use any callbacks it also shouldn't matter whether a lock or a rlock is used. (But actually also using a RLock as the machine context does not change the behaviour) Best regeards, Martin -- Dipl.-Ing. Martin Laabs Technische Universität Dresden Fakultät für Elektrotechnik und Informationstechnik Institut für Nachrichtentechnik, Lehrstuhl Hochfrequenztechnik Tel.: +49 (0)351 463-31918 Fax: +49 (0)351 463-37163 Email: martin.laabs@tu-dresden.de Please encrypt your mail if possible
aleneum commented 4 years ago

That is what I had expected and what I also relied on. But unfortunately this is not what I see. ... But actually also using a RLock as the machine context does not change the behaviour)

Just replacing Lock with RLock will indeed not change much. transitions used to use RLock directly but due to performance issues for deeply nested events we switched to a 'lighweight' version which facilitates Lock but only acquires the lock once for each thread and call (see #167). Without it self.to_A() in thread_run would never be executed because thread_run already acquired the lock. More precisely, the whole constructor would deadlock because it could not call other machine methods.

But let's say we override the thread check (or more accurately the assigning of the thread ID) after the constructor is done (see [1] below). You can (un)comment [1] to observe the difference in behaviour.

import time
from transitions.extensions import LockedMachine as Machine
import threading
import logging

class Test(Machine):
    states = ['A', 'B', 'C']

    def __init__(self):
        Machine.__init__(self, states=self.states,
                         initial='A'
                         )
        self.thread = threading.Thread(target=self.thread_run)
        Machine.__enter__ = lambda x: True  # [1]
        self.thread.start()

    def thread_run(self):
        logging.info('entering loop...')
        while True:
            time.sleep(1)
            self.to_A()  # this should spill quite some log messages

logging.basicConfig(level=logging.DEBUG)
t = Test()
#  >>> INFO:root:entering loop...
# that's it

I do not think that this is what you want to achieve either.

With my current understanding of your goal, I'd still insist this is a usage and design question where Stackoverflow is the more appropriate location to ask for assistance. If posting there does not help you or indeed a bug or lacking feature is identified I am more than happy to open this issue and working with you to solve it.