ChimeraTK / ControlSystemAdapter

An adapter layer which allows to use control applications with different control system software environments.
GNU Lesser General Public License v3.0
3 stars 2 forks source link

Fix persist data storage blocking the csa #22

Closed zenker closed 4 years ago

zenker commented 4 years ago

The latest version fixes #21. I'm not sure if we can accept removing the lock_guard and using the lock directly. Else handling the reference becomes a bit unhandy.

mhier commented 4 years ago

Exceptions are not properly handled like this. I don't understand why you don't want to use the lock_guard here. Just place it together with the read_latest() inside an extra scope:

{
  std::lock_guard<std::mutex> lock(_queueReadMutex);
  std::vector<DataType>& value = boost::fusion::at_key<DataType>(_dataMap.table)[id].read_latest();
}

read_latest() can throw exceptions, either if they are written to the queue (which we currently don't do, but how knows what we do in future), or it throws a std::system_error in case the semaphore has an error. The second is already a case we shouldn't ignore.

zenker commented 4 years ago

Ok so now I use the lock_guard. I was not sure if the object that corresponds to the reference returned by read_latest() still exists after the block with the lock_guard. But since it is a reference to the _dataMap it is ok to use the pointer after closing the block.

mhier commented 4 years ago

A reference never extends the life time of an object :-)

zenker commented 4 years ago

You should also add a new tag 01.05.03, since I changed the version in the CMakeLists.txt.

mhier commented 4 years ago

Since there are other changes we have to release the current master as 01.06.00. This needs to wait until we have fixed a few issues in ApplicationCore and I think the release should be done together.

zenker commented 4 years ago

Ok