Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-7753

CentroidChecker crashes with a segmentation violation.

    Details

    • Team:
      Data Release Production

      Description

      Running the attached script generates a SEGV. Please fix this, and check for similar errors elsewhere in meas_base.

       Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
      0   libmeas_base.dylib            	0x00000001150c83e0 lsst::meas::base::CentroidChecker::operator()(lsst::afw::table::SourceRecord&) const + 240
      1   libmeas_base.dylib            	0x00000001150cdd01 lsst::meas::base::GaussianCentroidAlgorithm::measure(lsst::afw::table::SourceRecord&, lsst::afw::image::Exposure<float, unsigned short, float> const&) const + 449
      2   _baseLib.so                   	0x0000000114d10fd6 _wrap_GaussianCentroidAlgorithm_measure(_object*, _object*) + 422
      

        Attachments

          Activity

          Hide
          jbosch Jim Bosch added a comment -

          I can reproduce this, but it's not immediately obvious what's going wrong; the code looks fine, and given that the abort is down in the depths of shared_ptr reference count atomics, I suspect it's a memory overrun of some sort happening elsewhere. Valgrinding from Python doesn't suggest anything obvious, so I'm going to see if I can reproduce this in pure C++ and valgrind there.

          Robert Lupton, if you have any (even wild) guesses from whatever preliminary debugging you did, please let me know.

          Show
          jbosch Jim Bosch added a comment - I can reproduce this, but it's not immediately obvious what's going wrong; the code looks fine, and given that the abort is down in the depths of shared_ptr reference count atomics, I suspect it's a memory overrun of some sort happening elsewhere. Valgrinding from Python doesn't suggest anything obvious, so I'm going to see if I can reproduce this in pure C++ and valgrind there. Robert Lupton , if you have any (even wild) guesses from whatever preliminary debugging you did, please let me know.
          Hide
          rhl Robert Lupton added a comment -

          I didn't do any forensics after I found where my notebook was crashing; sorry.

          Show
          rhl Robert Lupton added a comment - I didn't do any forensics after I found where my notebook was crashing; sorry.
          Hide
          jbosch Jim Bosch added a comment -

          Ok, figured it out - I was confused by the fact that (after optimized compilation) the offending line was actually after the one where the segfault occurred. The problem is that the CentroidChecker needs a peak attached to the Footprint to do its job. It's easy to check for that and hence turn this failure into an exception, and that's my first inclination. If you feel strongly that a centroider should be able to run without a Peak, I can simply have the CentroidChecker silently do nothing when there's no peak instead; that's a little less safe, but it may be more intuitive for anyone using our centroiders outside of their usual pipeline context.

          Show
          jbosch Jim Bosch added a comment - Ok, figured it out - I was confused by the fact that (after optimized compilation) the offending line was actually after the one where the segfault occurred. The problem is that the CentroidChecker needs a peak attached to the Footprint to do its job. It's easy to check for that and hence turn this failure into an exception, and that's my first inclination. If you feel strongly that a centroider should be able to run without a Peak , I can simply have the CentroidChecker silently do nothing when there's no peak instead; that's a little less safe, but it may be more intuitive for anyone using our centroiders outside of their usual pipeline context.
          Hide
          rhl Robert Lupton added a comment -

          I think an exception would be fine. The fixed code is:

          src = table.makeRecord()
          foot = afwDet.Footprint(exp.getBBox())
          foot.addPeak(centerX, centerY, peakVal)
          src.setFootprint(foot)
          

          Show
          rhl Robert Lupton added a comment - I think an exception would be fine. The fixed code is: src = table.makeRecord() foot = afwDet.Footprint(exp.getBBox()) foot.addPeak(centerX, centerY, peakVal) src.setFootprint(foot)
          Hide
          jbosch Jim Bosch added a comment -

          Ready for review; all code is in the linked meas_base PR.

          Show
          jbosch Jim Bosch added a comment - Ready for review; all code is in the linked meas_base PR.
          Hide
          jbosch Jim Bosch added a comment -

          Merged to master (review was marked as complete in GitHub PR).

          Show
          jbosch Jim Bosch added a comment - Merged to master (review was marked as complete in GitHub PR).

            People

            • Assignee:
              jbosch Jim Bosch
              Reporter:
              rhl Robert Lupton
              Reviewers:
              Robert Lupton
              Watchers:
              Jim Bosch, Robert Lupton
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel