Memory leak related to `get_secondary_event`

[akorgor]

When simulating models which involve secondary events a major continuous memory build-up is observable. The reason is a memory leak as detected by Valgrind related to the get_secondary_event method:

==29413== 1,120 bytes in 10 blocks are definitely lost in loss record 842 of 1,038
==29413==    at 0x4846FA3: operator new(unsigned long) (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==29413==    by 0x723966D2:
nest::eprop_learning_signal_connection_bsshslm_2020<nest::TargetIdentifierPtrRport>::get_secondary_event()
(eprop_learning_signal_connection_bsshslm_2020.h:230)
==29413==    by 0x72395FD1:
nest::GenericConnectorModel<nest::eprop_learning_signal_connection_bsshslm_2020<nest::TargetIdentifierPtrRport>
> ::get_secondary_event() (connector_model.h:205)
==29413==    by 0x721E1227: nest::ModelManager::get_secondary_event_prototype(unsigned int, unsigned
long) (model_manager.h:326)
==29413==    by 0x722612E5:
nest::SourceTable::compute_buffer_pos_for_unique_secondary_sources(unsigned long, std::map<unsigned
long, unsigned long, std::less<unsigned long>, std::allocator<std::pair<unsigned long const,
unsigned long> > >&) (source_table.cpp:249)
==29413==    by 0x721DC858:
nest::ConnectionManager::compute_compressed_secondary_recv_buffer_positions(unsigned long)
(connection_manager.cpp:1515)
==29413==    by 0x721CC5E3: nest::SimulationManager::update_connection_infrastructure(unsigned long)
(simulation_manager.cpp:770)
==29413==    by 0x721CD300: nest::SimulationManager::prepare() [clone ._omp_fn.0]
(simulation_manager.cpp:552)
==29413==    by 0x725A1CAB: GOMP_parallel (parallel.c:178)
==29413==    by 0x721CAE8F: nest::SimulationManager::prepare() (simulation_manager.cpp:549)
==29413==    by 0x721B8CFA: nest::KernelManager::prepare() (kernel_manager.cpp:110)
==29413==    by 0x721B6520: nest::prepare() (nest.cpp:267)

==29413== 4,368 bytes in 39 blocks are definitely lost in loss record 976 of 1,038
==29413==    at 0x4846FA3: operator new(unsigned long) (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==29413==    by 0x723966D2:
nest::eprop_learning_signal_connection_bsshslm_2020<nest::TargetIdentifierPtrRport>::get_secondary_event()
(eprop_learning_signal_connection_bsshslm_2020.h:230)
==29413==    by 0x72395FD1:
nest::GenericConnectorModel<nest::eprop_learning_signal_connection_bsshslm_2020<nest::TargetIdentifierPtrRport>
> ::get_secondary_event() (connector_model.h:205)
==29413==    by 0x721E1227: nest::ModelManager::get_secondary_event_prototype(unsigned int, unsigned
long) (model_manager.h:326)
==29413==    by 0x721DD40F: nest::ConnectionManager::deliver_secondary_events(unsigned long, bool,
std::vector<unsigned int, std::allocator<unsigned int> >&) (connection_manager.cpp:1690)
==29413==    by 0x7222D381: nest::EventDeliveryManager::deliver_secondary_events(unsigned long,
bool) (event_delivery_manager.cpp:351)
==29413==    by 0x721CE46E: nest::SimulationManager::update_() [clone ._omp_fn.0]
(simulation_manager.cpp:868)
==29413==    by 0x725A1CAB: GOMP_parallel (parallel.c:178)
==29413==    by 0x721CC8F5: nest::SimulationManager::update_() (simulation_manager.cpp:840)
==29413==    by 0x721CC1E0: nest::SimulationManager::call_update_() (simulation_manager.cpp:720)
==29413==    by 0x721CBB47: nest::SimulationManager::run(nest::Time const&) (simulation_manager.cpp:650)
==29413==    by 0x721B645C: nest::run(double const&) (nest.cpp:261)

The problem is that secondary connection events are created via new but never never deallocated:

https://github.com/nest/nest-simulator/blob/09091994902b091cbd74e4263830db10d5f25449/models/rate_connection_delayed.h#L176C1-L181C2

template < typename targetidentifierT >
SecondaryEvent*
rate_connection_delayed< targetidentifierT >::get_secondary_event()
{
  return new DelayedRateConnectionEvent();
}

As suggested by @heplesser the memory leak can be quickly patched by adding delete &prototype; in https://github.com/nest/nest-simulator/blob/09091994902b091cbd74e4263830db10d5f25449/nestkernel/connection_manager.cpp#L1703 after the while loop.

After this, the major memory build-up vanishes. However, there remains a small but continuous memory build-up

get_secondary_event is also used elsewhere especially through get_secondary_event_prototype. When manually deleting all the secondary event objects like in https://github.com/nest/nest-simulator/commit/e3b0cd179c84554583cab83c691aeba249391492 Valgrind does detect any memory leaks related to get_secondary_event anymore. However, the simulation is slower and the memory usage is constant at 19 GiB for the same simulation as shown in the previous image.

[med-ayssar]

Did you try using a unique_ptr maybe, it will auto clean itself while keeping the same previous structure. But be aware that you might need to use std::move to make the compiler happy.

[github-actions[bot]]

Issue automatically marked stale!