Moved management of "most_recent_ref_trigger_meta_" to SampleMetaData
and removed option to change RefMetaTriggerEvents
during runtime.
This will prevent a crash where several buffer managers read/write std::shared_ptr<SampleMetadata> lastRefMetaTrigger_;
To be cherry-picket to 7.0.0 acc7 branch and to be modified to apply on 6.1.x branch.
This change as well will require minimal changes on the FESA class to compile. No separate method "setRefTriggerEvents" anymore, removed possibility to change them during runtime --> Fesa Property ConfigureRefTriggerEvents
now is obsolete and can be dropped.
Merge request reports
Activity
added Bug label
requested review from @m.marn
All tests green now, ready for review @m.marn (gna, I have to work on a gitlab runner soon)
- Resolved by al.schwinn
- Resolved by al.schwinn
- Resolved by al.schwinn
- Resolved by al.schwinn
- Resolved by al.schwinn
- Resolved by al.schwinn
- Resolved by al.schwinn
Alright, it looks line the functionality was simply moved to the MetadataBuffer. As far as I can see it should be fine.
However the new methods are not locked as the others are. So I would suggest to have a thread-safe solution to lock the functions which must be public (
isRefMetaTriggerEvent
) and make the others private (isRefMetaTrigger
). Taking care that the call withinisRefMetaTrigger
does not call the publicisRefMetaTriggerEvent
but instead a non-locked private (lets sayisRefMetaTriggerEventHelper
) to not have a deadlock. The public lockedisRefMetaTriggerEvent
would then also call theisRefMetaTriggerEventHelper
only with the lock.I guess probably the current usage of the MetadataBuffer isn't multi-threaded, but having some functions thread-safe and not others makes it confusing, that's why I would suggest a unification.
I added these as code suggestions with the review. However I did it all through gitlab so I'm not completely sure I didn't mistype anything or miss anything.
I also see this MR doesn't include the changes from branch https://git.gsi.de/acc/cabad/-/tree/null-checks which also includes locks for two of the functions that miss them and some other fixes. I believe those should also be part of this "prevent crashing" fixes.