Skip to content
Snippets Groups Projects

Moved management of "most_recent_ref_trigger_meta_" to SampleMetaData

Merged al.schwinn requested to merge move_shared_resource_to_fix_crash into master

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

Approval is optional

Merged by al.schwinnal.schwinn 1 year ago (Jul 14, 2023 12:47pm UTC)

Merge details

  • Changes merged into master with 30244fc7 (commits were squashed).
  • Deleted the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • al.schwinn resolved all threads

    resolved all threads

    • Developer
      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 within isRefMetaTrigger does not call the public isRefMetaTriggerEvent but instead a non-locked private (lets say isRefMetaTriggerEventHelper) to not have a deadlock. The public locked isRefMetaTriggerEvent would then also call the isRefMetaTriggerEventHelper 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.

  • al.schwinn added 1 commit

    added 1 commit

    • 5381cf6f - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • al.schwinn added 1 commit

    added 1 commit

    • e11efe93 - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • al.schwinn added 1 commit

    added 1 commit

    • 9f2d8113 - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • al.schwinn resolved all threads

    resolved all threads

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading