Skip to content

Conversation

@MRiganSUSX
Copy link
Contributor

This PR adds additional checks for pointers insides a custom map.

This addresses issue #94

@emanuele-villa
Copy link
Member

emanuele-villa commented Mar 21, 2025

I set up dunetrigger to be in the same state that was throwing the error, but unfortunately this check does not raise a more graceful error than a segfault. It seems that for (const auto& pair : makers) already throws a core dump, even before checking validity of what's inside the map.

Note that I replaced the throw with a simple cerr, since in offline we cannot use Issues.hpp. I think that shouldn't change things, but correct me in case.

std::unique_ptr<T> AbstractFactory<T>::build_maker(const std::string& alg_name)
{
  std::cout << "Building maker " << alg_name << std::endl;
  creation_map& makers = get_makers();
  std::cout << "After creating map" << std::endl;
  
  for (const auto& pair : makers) {
    std::cout << "Entered loop of the check" << std::endl;
    if (!pair.second) {  // nullptr or invalid
      std::cerr <<  "Invalid state in the " << alg_name << " map, possibly not initialized" << std::endl;
      // throw FactoryInvalidState(ERS_HERE, alg_name);
    }
  }
  std::cout << "After check" << std::endl;
  auto it = makers.find(alg_name);

Output of execution:

Building maker TriggerCandidateMakerADCSimpleWindowPlugin
Getting makers
Returning...
After creating map
/afs/cern.ch/work/e/evilla/private/dune/dunesw/dunesw-config/scripts/protoduneana.sh: line 188:  7694 Segmentation fault    

@emanuele-villa
Copy link
Member

So I tried something not elegant at all (possibly just wrong), but the behavior changes. If I change the check lines to be:

        if (!makers.at(0)) {  // nullptr or invalid
              std::cerr <<  "Invalid state in the " << alg_name << " map, possibly not initialized" << std::endl;
                   // throw FactoryInvalidState(ERS_HERE, alg_name);
        }

I get

---- StdException BEGIN
  TriggerCandidateMakerTPC/tcmakerTPC during beginJob basic_string: construction from null is not valid
---- StdException END
%MSG
Art has completed and will exit with status 1.
 Reconstruction failed. Exiting...

Which is better, even though I don't understand why the cerr is not printed (maybe art replaces cerr with an art message?).

But I don't know, maybe it's such a particular situation that will never happen in the DAQ, or in the offline (unless includes go missing again for some weird reason) and it's not worth spending more time on it.

@MRiganSUSX
Copy link
Contributor Author

Hi @emanuele-villa, thanks for the additional details.
There will be a few comments, so I'll number them to make it easier to reference.

Your solution is not applicable in this case.
The !makers.at(0) is accessing the key == 0 (ie not the first key, but the key whose value is actually 0). The map is not expected to have such a key, as it holds the makers by their string names (ie TAMakerPrescaleAlgorithm). Or, to be exact, that line would always cause the std::cerr, regardless of whether the map is actually full/empty or not.

Another thing I wanted to check is whether the map is empty. However, this is not an actual issue, as the code covers the case when the map is empty (same case actually as if the passed algo was not found in the map) and the passed algo is added to the map, so this case does not need covering with an exception.
Basically, either the passed algo is not in the map and gets added; or it is and there is a throw for that. No other option can happen. It is expected for the map to be empty add the start, and then the algos get registered.

You mentioned that in offline we cannot use Issues.hpp. Why is this the case? The issues file is within the triggeralgs repository, and the only dependencies are ers and logging (triggeralgs is supposed to be mostly standalone). Does the offline not have this? How does it build?
The issue is that using ers is a coding style requirement for developing dunedaq code, which triggeralgs must adhere to. So this may be a bigger problem than it seems.

I'm happy to give this another go, but would you be able to share the setup that reproduces this issue please? Maybe the issue is before this step completely.

@MRiganSUSX
Copy link
Contributor Author

For completeness, this is how the code handles the registration. It starts empty, and one-by-one registers all available algos:

2025-Mar-28 13:56:08,044 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerCandidateMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerCandidateMaker, std::default_delete<triggeralgs::TriggerCandidateMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:29] Size: 0
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerActivityMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerActivityMaker, std::default_delete<triggeralgs::TriggerActivityMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:29] Size: 0
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerCandidateMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerCandidateMaker, std::default_delete<triggeralgs::TriggerCandidateMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:29] Size: 1
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerCandidateMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerCandidateMaker, std::default_delete<triggeralgs::TriggerCandidateMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:37] TCMakerChannelAdjacencyAlgorithm
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerCandidateMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerCandidateMaker, std::default_delete<triggeralgs::TriggerCandidateMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:38] 0x2444268
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerActivityMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerActivityMaker, std::default_delete<triggeralgs::TriggerActivityMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:29] Size: 1
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerActivityMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerActivityMaker, std::default_delete<triggeralgs::TriggerActivityMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:37] TAMakerChannelAdjacencyAlgorithm
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerActivityMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerActivityMaker, std::default_delete<triggeralgs::TriggerActivityMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:38] 0x2443fd8
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerCandidateMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerCandidateMaker, std::default_delete<triggeralgs::TriggerCandidateMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:29] Size: 2
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerCandidateMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerCandidateMaker, std::default_delete<triggeralgs::TriggerCandidateMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:37] TCMakerBundleNAlgorithm
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerCandidateMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerCandidateMaker, std::default_delete<triggeralgs::TriggerCandidateMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:38] 0x2446168
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerCandidateMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerCandidateMaker, std::default_delete<triggeralgs::TriggerCandidateMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:37] TCMakerChannelAdjacencyAlgorithm
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerCandidateMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerCandidateMaker, std::default_delete<triggeralgs::TriggerCandidateMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:38] 0x2444268
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerActivityMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerActivityMaker, std::default_delete<triggeralgs::TriggerActivityMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:29] Size: 2
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerActivityMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerActivityMaker, std::default_delete<triggeralgs::TriggerActivityMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:37] TAMakerBundleNAlgorithm
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerActivityMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerActivityMaker, std::default_delete<triggeralgs::TriggerActivityMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:38] 0x2446098
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerActivityMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerActivityMaker, std::default_delete<triggeralgs::TriggerActivityMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:37] TAMakerChannelAdjacencyAlgorithm
2025-Mar-28 13:56:08,045 LOG [static void triggeralgs::AbstractFactory<T>::register_creator(std::string, maker_creator) [with T = triggeralgs::TriggerActivityMaker; std::string = std::__cxx11::basic_string<char>; maker_creator = std::function<std::unique_ptr<triggeralgs::TriggerActivityMaker, std::default_delete<triggeralgs::TriggerActivityMaker> >()>] at /nfs/home/mrigan/dune-daq/v5.3-250328/sourcecode/triggeralgs/include/triggeralgs/AbstractFactory.hxx:38] 0x2443fd8

@ArturSztuc
Copy link
Contributor

@MRiganSUSX & @emanuele-villa what is the status of this PR, and the related issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants