POETSII / Orchestrator

The Orchestrator is the configuration and run-time management system for POETS platforms.
1 stars 1 forks source link

RTCL uses un-sychronised cross-thread comms #237

Closed m8pple closed 3 years ago

m8pple commented 3 years ago

RTCL uses a set of variables in a shared struct:

https://github.com/POETSII/Orchestrator/blob/372ccc251832256a33c59bb3c80c2199efac298a/Source/RTCL/RTCL.h#L27-L35

One thread is reading from this struct :

https://github.com/POETSII/Orchestrator/blob/372ccc251832256a33c59bb3c80c2199efac298a/Source/RTCL/RTCL.cpp#L15-L25

And another thread is writing to it :

https://github.com/POETSII/Orchestrator/blob/372ccc251832256a33c59bb3c80c2199efac298a/Source/RTCL/RTCL.cpp#L140-L150

However, there is no threading synchronisation, so (assuming that those variables ever do change - I'm not sure), there is no guarantee that any changes will ever get migrated from one thread to another. There needs to be use of atomics or mutexes to actually ensure that hardware will always migrate, and to ensure that the compiler doesn't make optimisation assumptions.

Probably it's working by chance at the moment, but it might stop at any point.

heliosfa commented 3 years ago

@DrongoTheDog "Yes, there is a comms pool in the middle. What is the problem with that?"

"In general you are correct, but the reading thread is spinning so will pick up any change in future iterations."

This does away with the need for atomics/mutexes as we have the constant spinner any way (for design reasons)

m8pple commented 3 years ago

I think you're assuming that writes from one thread will automatically appear on another thread, but the C memory model doesn't necessarily require that. So you're in danger of the optimiser re-writing the code into an infinite loop.

However, I don't really care enough that I'll fix it myself, so I'll just close the issue.

DrongoTheDog commented 3 years ago

On 22/06/2021 16:07, m8pple wrote:

CAUTION: This e-mail originated outside the University of Southampton.

I think you're assuming that writes from one thread will automatically appear on another thread, but the C memory model doesn't necessarily require that. So you're in danger of the optimiser re-writing the code into an infinite loop.

I didn't know that. Seriously? Explain.....

However, I don't really care enough that I'll fix it myself, so I'll just close the issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FPOETSII%2FOrchestrator%2Fissues%2F237%23issuecomment-866066204&data=04%7C01%7Cadb%40soton.ac.uk%7Ca2062f7e9ee94af9a51208d9358f7f70%7C4a5378f929f44d3ebe89669d03ada9d8%7C0%7C0%7C637599712722749613%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=%2FkNpyVjyNCkKvarNP5mKH%2FGbxyLLOc4ut37TodRHOPk%3D&reserved=0, or unsubscribe https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAE7EYI5UXFRISQ6ELHIMN4TTUCRMHANCNFSM46U5OB5A&data=04%7C01%7Cadb%40soton.ac.uk%7Ca2062f7e9ee94af9a51208d9358f7f70%7C4a5378f929f44d3ebe89669d03ada9d8%7C0%7C0%7C637599712722759607%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=0OAj5O1kuFxgjUvOyZohegyIz2xxGezpZuWWQPJOwpg%3D&reserved=0.

--

Andrew Brown Professor of Electronics University of Southampton Hampshire SO17 1BJ UK

T: 02380 593374 M: 07464 981171