Skip to content

Use unique_ptr< SecondaryEvent > to avoid memory leaks#3562

Merged
heplesser merged 14 commits intonest:masterfrom
heplesser:fix-3501
Sep 11, 2025
Merged

Use unique_ptr< SecondaryEvent > to avoid memory leaks#3562
heplesser merged 14 commits intonest:masterfrom
heplesser:fix-3501

Conversation

@heplesser
Copy link
Contributor

This PR fixes #3501 using the std::unique_ptr approach suggested by @med-ayssar in that issue.

@akorgor Could you test this with Valgrind? Could you also check against master that using unique_ptr does not slow down simulations?

@med-ayssar Could you review as well?

@heplesser heplesser added this to the NEST 3.9 milestone Sep 1, 2025
@heplesser heplesser requested a review from akorgor September 1, 2025 21:43
@heplesser heplesser added T: Bug Wrong statements in the code or documentation S: High Should be handled next I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Sep 1, 2025
@heplesser heplesser added this to Kernel Sep 1, 2025
@github-project-automation github-project-automation bot moved this to In progress in Kernel Sep 1, 2025
Copy link
Contributor

@med-ayssar med-ayssar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@akorgor
Copy link
Contributor

akorgor commented Sep 10, 2025

Comparing the Valgrind memory-leak logs of nest master to those of PR #3562, PR #3561, and
the combination of PRs #3561 and #3562 shows that these changes fix the memory leaks, leaving only SLI-related leaks.

@akorgor
Copy link
Contributor

akorgor commented Sep 11, 2025

Measuring runtime and memory usage against both the unpatched NEST version and a temporary fix using delete &prototype after lines

while ( lcid < lcid_end )
{
std::vector< unsigned int >::iterator readpos = recv_buffer.begin() + positions_tid[ syn_id ][ lcid ];
prototype << readpos;
prototype.set_stamp( stamp );
// send delivers event to all targets with the same source
// and returns how many targets this event was delivered to
lcid += connections_[ tid ][ syn_id ]->send( tid, lcid, cm, prototype );
}
shows that runtime remains unchanged while memory usage improves.

Copy link
Contributor

@akorgor akorgor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this valuable fix!

@heplesser heplesser merged commit 5e5096e into nest:master Sep 11, 2025
53 of 54 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in Kernel Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Bug Wrong statements in the code or documentation

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Memory leak related to get_secondary_event

3 participants

Comments