Project

General

Profile

Bug #22696

Unit tests fail after change to makeRoutingMasterPolicy return type

Added by Kurt Biery over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Known Issues
Target version:
Start date:
06/06/2019
Due date:
% Done:

0%

Estimated time:
Experiment:
-
Co-Assignees:
Duration:

Description

In the changes associated with Issue #22530, I changed the return type of makeRoutingMasterPolicy from a unique_ptr to a shared_ptr. However, I realize now that the new code did not correctly construct the shared_ptr that was returned.

This was uncovered by failures in the unit tests for RoundRobin_policy_t, NoOp_policy_t, and CapacityTest_policy_t. The failure modes had complaints about 'no privilege to access' elements of the RoutingPolicy at destruction time.

I now know the correct way to convert from the unique_ptr that is returned by art BasicPluginMaker to the shared_ptr that we want makeRoutingMasterPolicy to return. I will commit the change on a branch in a few minutes.

History

#1 Updated by Kurt Biery over 1 year ago

  • Status changed from Assigned to Resolved

I've committed the code to a branch called bugfix/22696_makeRMPolicy_SharedPtrFix in the artdaq repository.

In addition to the code change in makeRoutingMasterPolicy.cc, I added a set of tests in RoundRobin_policy_t to verify the behavior of the shared_ptr that is returned by makeRoutingMasterPolicy.

To verify these changes, I would suggest running the unit tests for the artdaq repo before and after the code change is included in a particular software development area.
As well as inspecting the code. Especially, the handling of the unique_ptr and shared_ptr in makeRoutingMasterPolicy.

#2 Updated by Eric Flumerfelt over 1 year ago

  • Status changed from Resolved to Reviewed
  • Co-Assignees Eric Flumerfelt added

I have reviewed and tested this change.

Based on my research, std::shared_ptr instances can be constructed directly from unique_ptrs (i.e. unique_ptr can decay into shared_ptr), so this change is overly-verbose.

std::shared_ptr<artdaq::RoutingMasterPolicy>
artdaq::makeRoutingMasterPolicy(std::string const& policy_plugin_spec,
                                                                                 fhicl::ParameterSet const& ps)
{
        static cet::BasicPluginFactory bpf("policy", "make");

        return bpf.makePlugin<std::unique_ptr<artdaq::RoutingMasterPolicy>,
                               fhicl::ParameterSet const &>(policy_plugin_spec, ps);
}

Would be just as effective at resolving the problem, but is definitely less readable, due to the mixing of unique_ptr in the makePlugin call and the shared_ptr return type. My vote is to leave the code as-is.

#3 Updated by Eric Flumerfelt over 1 year ago

  • Target version set to artdaq v3_06_00
  • Status changed from Reviewed to Closed
  • Category set to Known Issues

Also available in: Atom PDF