How to protect attribute write value with critical section?

Hi Tango dancers!

In Sardana project we just tracked the source of a very annoying bug that moves motors to a wrong positions (usually 0)

What we basically do is that in the server we have a single worker thread responsible for pushing all change events. Before pushing an event the worker thread updates the attribute write value using the set_write_value() method.

This collides with processing the attribute write request since the write value obtained with get_write_value() returns corrupted data (usually at least one zero in case of the spectrum attribute).

I imagine that only the call to push_change_event() and processing the attribute write request are protected by the Serialization Monitor, so the arbitrary calls to set_write_value() are not protected.

I tried to use my own lock but without success. Do you have any suggestion on how to deal with that?

My colleague Miquel Navarro created this reduced example to reproduce this issue: https://github.com/Guspyro/tango-attributes-random-zero.

Many thanks in advance for your help!
Zibi
Hi Zibi,

I think you could use the AutoTangoMonitor class.
I didn't find any documentation for this class but it seems to be exposed in pyTango.

The following seems to work for me:


def backgroundWrite(self):
sleep(1.0) # Let the device server initialize correctly
while self.runBackgroundWrite:
with AutoTangoMonitor(self):
self.get_device_attr().get_attr_by_name('Position').set_write_value([1.2, 3.4])
self.debug_stream("Set_write_value [1.2, 3.4]")
sleep(0.001)
Rosenberg's Law: Software is easy to make, except when you want it to do something new.
Corollary: The only software that's worth making is software that does something new.
Edited 1 month ago
For convenience, I created the following PR in the GIT repository you mentioned.

You can see the proposed patch here:

https://github.com/Guspyro/tango-attributes-random-zero/pull/1/files
Rosenberg's Law: Software is easy to make, except when you want it to do something new.
Corollary: The only software that's worth making is software that does something new.
Thanks Reynald!
Thanks Reynald! I tried your suggestion with the reduced example and it avoids the data corruption! We will check it with Sardana soon in the systems where we were suffering this issue.

Before knowing that we (device classes developers) can take the Tango Serialization Monitor I was thinking on another possible solution to this problem: adding new variants to push_change_event() methods (in cppTango & PyTango) where one could pass the w_value. Just out of curiosity, what do you think about it?
zreszela
Before knowing that we (device classes developers) can take the Tango Serialization Monitor I was thinking on another possible solution to this problem: adding new variants to push_change_event() methods (in cppTango & PyTango) where one could pass the w_value. Just out of curiosity, what do you think about it?

I think it's a good idea because it's a common use case.
Rosenberg's Law: Software is easy to make, except when you want it to do something new.
Corollary: The only software that's worth making is software that does something new.
Hi Reynald,

I have a doubt regarding fixing it in Sardana, and more preciselly how broad should be the critical section. I think that protecting only call to the set_write_value() is not enough and the critical section should also include call to the push_change_event(). Otherwise, if the worker thread handling the _set_attribute_push() gets preempted (during the context switching) in favor of another thread handing the same attribute write request, then, when the worker thread would be finally resumed the write value would be already modified and incorrect. Is this reasoning correct? Note that the first, option already avoids the wrong moves (when the array were corrupted with zeros).

Cheers!
zreszela
Is this reasoning correct? Note that the first, option already avoids the wrong moves (when the array were corrupted with zeros).
Cheers!

Your reasoning seems correct to me.
Rosenberg's Law: Software is easy to make, except when you want it to do something new.
Corollary: The only software that's worth making is software that does something new.
Hi again,

Should I create a feature request to cppTango for the new variants of push_change_event()?
zreszela
Hi again,

Should I create a feature request to cppTango for the new variants of push_change_event()?

Yes, please!
Rosenberg's Law: Software is easy to make, except when you want it to do something new.
Corollary: The only software that's worth making is software that does something new.
 
Register or login to create to post a reply.