# CentroidChecker crashes with a segmentation violation.

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• 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 const&) const + 449 2 _baseLib.so 0x0000000114d10fd6 _wrap_GaussianCentroidAlgorithm_measure(_object*, _object*) + 422 

## Attachments

1. bug.py
1 kB
Robert Lupton

## Activity

Hide
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
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
Robert Lupton added a comment -

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

Show
Robert Lupton added a comment - I didn't do any forensics after I found where my notebook was crashing; sorry.
Hide
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
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
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
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
Jim Bosch added a comment -

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

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

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

## People

• Assignee:
Jim Bosch
Reporter:
Robert Lupton
Reviewers:
Robert Lupton
Watchers:
Jim Bosch, Robert Lupton