Ticket #10037 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Sum spectra then sum neighbours causes crash on EnginX

Reported by: Owen Arnold Owned by: Harry Jeffery
Priority: critical Milestone: Release 3.3
Component: Framework Keywords: Student
Cc: Blocked By:
Blocking: Tester: Owen Arnold

Description

Apparently, SummingSpectra then running SumNeighbours causes a crash. This seems like a silly thing to try and do, but Mantid should not crash as a result.

Change History

comment:1 Changed 6 years ago by Owen Arnold

  • Summary changed from Sum spectra then sum neighbours causes crash on EngineX to Sum spectra then sum neighbours causes crash on EnginX

comment:2 Changed 6 years ago by Nick Draper

  • Owner changed from Anyone to Harry Jeffery
  • Status changed from new to assigned

comment:3 Changed 6 years ago by Harry Jeffery

  • Status changed from assigned to inprogress

comment:4 Changed 6 years ago by Harry Jeffery

Fix crash when running SumNeighbours algorithm.

Refs #10037.

Changeset: 61385ea927c891f9cc1c7b7d9e2136b8e566c864

comment:5 Changed 6 years ago by Harry Jeffery

Fix the build.

Refs #10037.

Changeset: b8f1a5bc64dda08fc22ad7b897658c9c5a9df994

comment:6 Changed 6 years ago by Harry Jeffery

  • Status changed from inprogress to verify
  • Resolution set to fixed

I had a little trouble reproducing this at first, but I found that running the script given in #10038, then running SumSpectra and SumNeighbours on the ENGINX workspace with default values triggered the crash (which just happens to be an unchecked pointer dereference).

With the changes applied the pointers are now checked properly before being dereferenced.

Testing: Verify the crash can no longer be reproduced.

Branch: ​https://github.com/mantidproject/mantid/compare/feature/10037_crash_on_sum_neighbours

The rect pointer is itself checked later on, and appropriate action is taken, so all we needed to do is leave rect as NULL on failure.

comment:7 Changed 6 years ago by Owen Arnold

  • Status changed from verify to verifying
  • Tester set to Owen Arnold

comment:8 Changed 6 years ago by Owen Arnold

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/10037_crash_on_sum_neighbours'

Full changeset: ​e975df970e5e51b1937f49e7d5db42743cb976ad

comment:9 Changed 6 years ago by Owen Arnold

This works fine now. For future reference. If you can get a regression test in there first, before you make the fix. That tends to form a nice overall solution.

comment:10 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​10879

Note: See TracTickets for help on using tickets.