Ticket #10037 (closed: fixed)
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:4 Changed 6 years ago by Harry Jeffery
Fix crash when running SumNeighbours algorithm.
Refs #10037.
Changeset: 61385ea927c891f9cc1c7b7d9e2136b8e566c864
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