Attempt to fix memory leak in AthenaOutputStreamTool

0fc6d9d1 from !67465 (merged) possibly causes the memory leak.

Hence, reverting it to attempt to fix ATLASG-2660

added full-integration-tests full-unit-tests labels

requested review from @gemmeren

Jenkins please retry the build

This merge request affects 1 package:

• Control/AthenaServices

This merge request affects 1 file:

• Control/AthenaServices/src/AthenaOutputStreamTool.cxx

Adding @gemmeren ,@ssnyder ,@maszyman as watchers

added Core main labels

Hi @maszyman ,

it's not clear to me, why 0fc6d9d1 would introduce such a sizeable memory leak, but I have no objections to reverting it. IIRC, this all started with the attempt to fix a very minor (once per job) memory leak :(. I am very surprised that we did not see this during CI or SPOT testing, so we need to make sure that reverting 0fc6d9d1 remedies the situation.

Thanks, Peter

CI Result SUCCESS (hash 6760b8b5)

	Athena	AthSimulation	AthGeneration	AthAnalysis
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 3185]

45,255,933 (19,916,800 direct, 25,339,133 indirect) bytes in 77,800 blocks are definitely lost in loss record 59,864 of 59,864
   at 0x484AF01: operator new(unsigned long) (vg_replace_malloc.c:434)
   by 0x35F1C448: AthenaOutputStreamTool::streamObjects(std::vector<DataObject*, std::allocator<DataObject*> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (AthenaOutputStreamTool.cxx:448)
   by 0x35F06765: AthenaOutputStream::write() (AthenaOutputStream.cxx:641)
   by 0x35EFB067: AthenaOutputStream::execute() (AthenaOutputStream.cxx:532)

That's where we're allocating the new TokenAddress and subsequently leak it. It seems to scale linearly w/ the number of events. I haven't checked if this MR fixes the issue but we can also revert the whole MR.

approved this merge request

Hi @maszyman , @amete ,

It's rather urgent to fix the memory leak. Do you feel comfortable enough to un-draft this MR and get it into the nightly?

Thanks, Peter

It looks much better:

I'm going to resolve the threads and push this through.

resolved all threads

marked this merge request as ready

added review-pending-level-1 label

added bugfix label

Hi @amete, I'm gonna take it as a green light and merge it right away

added review-approved label and removed review-pending-level-1 label

merged

mentioned in commit c90ae0d5

Thanks a lot @tsulaia!

changed the description

Thank you very much for proceeding with this!

mentioned in merge request !68171 (merged)

mentioned in commit e62ff396