Skip to content

Draft: Safe interworking between read_attribute() and push_event()

This PR fixes two crashes reported in #443:

  1. When Attr1 and Attr2 are read in single read_attributes({Attr1, Attr2}) and Attr2 pushes change event for Attr1 (order of attribute names passed to read_attributes is important here)
  2. When attribute pushes a change event for itself from read callback after calling set_value.

In the first case, when Attr2 read callback pushes an event for Attr1, it overwrites and then releases CORBA sequence associated with Attr1. This is because sequence obtained from Attr1 read callback is not yet copied for network transfer when Attr2 read callback is called, i.e. it worked like this pseudocode:

for(Attribute& a : attributes) { invoke_read_callback(a); }
for(Attribute& a : attributes) { store_sequence(a); }

This PR changes the behavior to be like:

for(Attribute& a : attributes) { invoke_read_callback(a); store_sequence(a); }

This resolved the issue and allowed to revert correction for #241 (closed) which attempted to fix the same problem by keeping value flag set to true after deleting the sequence. Correction for #241 (closed) (330a68c6) worked only for scenarios where attribute that pushed event was read first and resulted in a crash when attribute reading order was inverted.

In the second case, when push_event is called after set_value, we now get API_AttrValueNotSet exception instead of a crash. This is just result of reverting previous #241 (closed) correction. We cannot really 'fix' this scenario and make it work as one would expect because push_event and set_value are operating on the same buffer and push_event automatically releases that buffer when it is done with pushing the event. See also https://github.com/tango-controls/cppTango/issues/443#issuecomment-569268014.

I also took the opportunity to refactor whole Device_3Impl::read_attributes_no_except and split it into more manageable chunks.

Fixes #443, #241 (closed).

Edited by Thomas Juerges

Merge request reports