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?
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)
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?
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).
[quote=“zreszela”]Is this reasoning correct? Note that the first, option already avoids the wrong moves (when the array were corrupted with zeros).
Cheers![/quote]