Skip to content

Fix host and device memory leaks in module setup and teardown - #1995

Draft
sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:fix/memory-leaks
Draft

sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:fix/memory-leaks

Conversation

@sbryngelson

Copy link
Copy Markdown
Member
  • m_surface_tension: c_divs has num_dims + 1 fields but finalize freed
    only num_dims of them, leaking the gradient-magnitude field.
  • m_cbc: pi_coef_y/z are allocated when weno_order > 1 or
    muscl_order > 1 but were freed only for weno_order > 1, leaking them
    in MUSCL runs with characteristic BCs. Use the x-direction condition.
  • m_global_parameters (pre/sim/post): MPI_IO_DATA%var(i)%sf and
    MPI_IO_IB_DATA%var%sf were allocated and then immediately nullified,
    leaking every buffer. The pointers are default-initialized to null and
    are associated later by s_initialize_mpi_data, so drop the allocate
    loops. Free MPI_IO_DATA%var/view unconditionally in finalize to match
    the unconditional allocation (they leaked when parallel_io was off).
  • m_global_parameters (sim): neighbor_ranks was freed after the n == 0
    and p == 0 early returns, so 1D/2D runs leaked it. Free it first.
  • m_data_output (sim): the file_per_process path called
    s_initialize_mpi_data a second time (identical pointers, leaked MPI
    derived types) and created the restart directory twice. Down-sampled
    output writes q_cons_temp_ds directly, and down_sample requires IGR,
    which excludes Euler bubbles, so the second call had no effect.

No change to results: the debug CPU test suite passes unchanged, and
2-rank NVHPC MPI runs with parallel_io (2D IBM with a shared file and
with file_per_process; 3D IGR file_per_process with and without
down_sample) give byte-identical restart data and identical silo output
(h5diff) against master.


Acknowledgement

  • I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

- m_surface_tension: c_divs has num_dims + 1 fields but finalize freed
  only num_dims of them, leaking the gradient-magnitude field.
- m_cbc: pi_coef_y/z are allocated when weno_order > 1 or
  muscl_order > 1 but were freed only for weno_order > 1, leaking them
  in MUSCL runs with characteristic BCs. Use the x-direction condition.
- m_global_parameters (pre/sim/post): MPI_IO_DATA%var(i)%sf and
  MPI_IO_IB_DATA%var%sf were allocated and then immediately nullified,
  leaking every buffer. The pointers are default-initialized to null and
  are associated later by s_initialize_mpi_data, so drop the allocate
  loops. Free MPI_IO_DATA%var/view unconditionally in finalize to match
  the unconditional allocation (they leaked when parallel_io was off).
- m_global_parameters (sim): neighbor_ranks was freed after the n == 0
  and p == 0 early returns, so 1D/2D runs leaked it. Free it first.
- m_data_output (sim): the file_per_process path called
  s_initialize_mpi_data a second time (identical pointers, leaked MPI
  derived types) and created the restart directory twice. Down-sampled
  output writes q_cons_temp_ds directly, and down_sample requires IGR,
  which excludes Euler bubbles, so the second call had no effect.

No change to results: the debug CPU test suite passes unchanged, and
2-rank NVHPC MPI runs with parallel_io (2D IBM with a shared file and
with file_per_process; 3D IGR file_per_process with and without
down_sample) give byte-identical restart data and identical silo output
(h5diff) against master.
@github-actions

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/simulation/m_global_parameters.fpp 764 -31
src/post_process/m_global_parameters.fpp 392 -22
src/pre_process/m_global_parameters.fpp 468 -19
src/simulation/m_data_output.fpp 1576 -2
Directory Lines Diff
pre_process 5015 -19
simulation 28412 -33
post_process 3477 -22
total 47352 -74

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant