Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible race condition when stopping a resource #277

Open
cochicde opened this issue Nov 19, 2024 · 4 comments
Open

Possible race condition when stopping a resource #277

cochicde opened this issue Nov 19, 2024 · 4 comments

Comments

@cochicde
Copy link
Contributor

One case when the problem occurs is when starting a resource and stopping it right afterwards, because some events are still running when the FB are changed to a stopped stated.

The case I encountered was specifically in the BaseCommFB, where the INIT event was being executed and therefore the connection is being opened, and then the changeExecutionState(stop/kill) call arrives from another thread (usually the main thread) which closes the closes the connection deleteing the topComStack and causing issues.

I think the problem is in the CResource::changeExecutionState where the mResourceEventExecution is stopped only after all the internal FBs are stopped. I believe we should change the states of the internal of the FBs only when we are sure that the mResourceEventExecution is not running anymore, meaning that the it should be stopped first, the ecet itself should first join its thread before returning and only then the FBs should be stopped.

So, since the resource is a composition of a ecet and internal FBs, the order of the start of it should be:

  1. FBs
  2. ecet

and the order of the stop of it should be the opposite:

  1. ecet
  2. FBs

stopping (and probably starting too) the ecet need to be synchronous to actually make sure that the ecet is in the desired state before continuing, otherwise you end up with a transit state where you triggered the change of the state, but it was not reached yet.

@cochicde
Copy link
Contributor Author

I see now that the E_RESTART FB is handling the Start/Stop state changes in close relationship with the ecet, which would also require some re-thinking

@cochicde
Copy link
Contributor Author

After some thought, my doubts are:

  • what's the point of triggering the STOP event output of RESTART when the rest of the FBs in the resource will be in stopped mode anyway?
  • If the trigger would make sense if only the RESTART is set to stop, maybe RESTART should check first if the ecet is still alive before trying to trigger the STOP event, otherwise just go into STOP state and return.

@azoitl
Copy link
Contributor

azoitl commented Nov 19, 2024

The description of the E_RESTART block is a bit vague in the standard. For years we have tried to come up with different interpretations how to correctly implement it. Your assessment helps that we can come to a better solution. This will also be important for the thing @MandKastner is currently to start working at.

What I can do is to tell my understanding of E_RESTART: For me it is a means that allows to inform FB Networks in this resource that the execution of this resource has started or is stopped. The events it sends out shall be used to trigger any initialization initialization. Therefore I think that E_RESTART instances should be started and stopped separately to the FBs so that the triggered FBs can be informed and execute. For starting this is less critical as any output events that an E_RESTART block is sending are put into the ECET. So it is fine to first START all blocks and then the ECET.

For STOPPING I think we need to first STOP E_RESTART, wait till all execution because of the STOP events completes. Stop ECET and then STOP all the FBS. Or did I miss anything.

@cochicde
Copy link
Contributor Author

cochicde commented Nov 24, 2024

I see. I agree with the stopping procedure, but now I have more thoughts :)

  1. How do we know when the STOP event chain finished? Could we create a temporary ecet for this?(maybe this would be even more problematic regarding race conditions). What if there's a loop somehow in this STOP event chain? Should we see this as a user error which can't be handled by the runtime? Do we use the KILL command for immediate stop of the ecet and FBs activity? I'd see then KILL kind of a force STOP.
  2. After the STOP event fron E_RESTART is triggered, how do handle the external events still coming in? Should we shut down the input from the system to the resource, meaning not allowing new event chains? (I'm sure if this could be problematic for some use cases).
  3. I see some overlap between the Start/Stop states which are triggered by the "system", and the init/deinit transitions which are triggered by events (usually triggered by the START/STOP from E_RESTART). From your explanation and what I see in the code, the Stopped state of a Function Block inhibits any logic being executed in it (does not process any event and shuts down any connection to the system). However, after some thoughts, I don't see this as a problem actually, but a good strategy.

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

No branches or pull requests

2 participants