Skip to content

Fix JetSuper Roi constituent management

Mark Sutton requested to merge sutt/athena:r into 23.0

The JetSuper Roi was being told to not manage it's constituents, but then mistakenly, unique_ptr was being used to created those constituents, but of course, as they were being stored as standard pointers, they were being released first, so the unique pointer was no longer managing them, and so were subsequently leaking when the superRoi was being destroyed.

In principle the unique_ptr could have then been reset() with the now, standard ptr, but then the now standard constitent pointer in the Roi would not have been safe to use once the unique_ptr had gone out of scope - it would have still be there, but would the memory be reclaimed at some later stage ? So would it really be safe to use ? Probably not, and almost certainly calling delete on it would cause a malloc error as the address would have already been deleted when the unique_ptr went out of scope.

As such unique_ptr is inappropriate here, and so we resort back to good old "new" and let the Roi manage it's consituents itself.

In addition, copy constructors with explicit types are added - previously only a copy constructor and assignment with the base class was included so default constructors were being used, which only did a shallow copy, and hence when the copies of the superRoi were deleted (why were there copies anyhow ???) then the constituents were being deleted, such that they no longer existed when the TP convertor was trying to access them from the original superRoi.

Interestingly, when adding the extra constructors the code still compiled and ran without crashing without needing to recompile all the clients which include the TrigRoiDescriptor. Perhaps the vtable for the compiled classes was not changed because the default copy constructors were already there.

All Roi copies are deep copies of the constituents now, if manageConstituents is set correctly, so this would be guaranteed safe.

Should fix ATR-25990

Edited by Mark Sutton

Merge request reports

Loading