Skip to content

FNL: no device-memory teardown on any FNL type (~100 pointers never freed); leaks per-realm on the forest path #34

Description

@szaghi

Statement

No FNL type has device-memory teardown. Repo-wide, dev_free appears in production code at exactly two sites, both freeing function-local scratch:

  • src/app/prism/fnl/adam_prism_fnl_object.F90:4305dev_free(skin_gpu, mydev)
  • src/app/prism/fnl/adam_prism_fnl_object.F90:5258dev_free(delta_gpu, mydev)

Every other dev_free in the repo is under src/tests/. Zero of the 13 FNL object types declares a destroy, finalize, or final procedure (grep -rn "final ::|procedure.*destroy" src/lib/fnl/ src/app/prism/fnl/ returns nothing).

The only shutdown path is prism_fnl_object%finalize_forest (:4705-4713) — save_simulation_data + io%close_file_residuals, no device work — and finalize_mpi_forest (:4715-4724), which calls mpih_fnl%finalize.

Roughly 100 device pointers are allocated and never freed.

Inventory

Type Never-freed pointers Largest
field_fnl_object 5 x/y/z_cell_gpu, dxyz_gpu
maps_fnl_object 13 ghost/seam maps + buffers
ib_fnl_object 2 phi_gpu (5D, per solid)
rk_fnl_object 4 q_rk_gpu (nrkx field)
weno_fnl_object 7 cell_scheme_gpu, ror_stats_gpu (5D)
prism_fnl_object 8 flxyz_c_gpu (rank-7, 9x field)
prism_fnl_coil_object 4 j_vec_gpu (6D)
prism_fnl_pml_object 42 6x q_pml_*_gpu
prism_fnl_pic_object 3 q_pic_gpu
prism_fnl_leapfrog_pic_object 1 q_pic_old_gpu
prism_fnl_rk_pic_object 3 q_pic_rk_gpu
prism_fnl_rk_pml_object 15 6x q_pml_*_rk_gpu (nrkx face)

adam_fnl_mpih_object is a rename alias over fundal_mpih_object with no ADAM-side device state; it is the only FNL type with working teardown, via FUNDAL's own mpih_fnl%finalize.

The sharpest illustration is prism_fnl_pic_object: its three host staging buffers are allocatable and auto-freed by the language; its three device buffers are not. The teardown was simply never written.

Why it is latent today

Three things keep this from being fatal:

  1. prism_fnl_object%allocate_gpu — all 8 raw dev_allocs — is called once, at initialize_prism:379, outside the IC/AMR repeat loop.
  2. The repeat path (3 copy_cpu_gpu invocations at startup, via the loop at :4772-4777 plus :4784) touches only dev_assign_to_device targets, which free-before-realloc. This is deliberate — see the comment at :4778-4781.
  3. Runtime regrid is locked off on FNL: amr_locked_ = .true. at initialize_forest:4863, and the amr_update call in post_step_forest is commented out (:5304).

So on a single-realm run this is one generation leaked at exit — cosmetically bad, operationally harmless.

Why it is not harmless on the forest path

src/app/prism/fnl/adam_prism_fnl.F90:33 declares type(prism_fnl_object), allocatable :: realm(:), sized manifest%realms_number (:49) or 1 (:52). Each realm runs its own full initialize_prism, so every allocation in the table above happens N times and none is ever released.

Meanwhile initialize_prism:370 divides the per-device budget by realms_number:

memory_avail_ = real(mpih_fnl%dev_memory_avail/1e9, R8P) / real(realms_number_, R8P)

so each realm believes it has 1/N of the device. mpih_fnl is explicitly guarded against re-init (:363-367); device memory is not.

This makes the device high-water mark scale with N while the budget accounting does not — a plausible independent path to the size-dependent OOM tracked in the companion issue, and one that would show up on a cluster manifest with more realms than the local box.

Latent hazard: dev_alloc is not free-safe

dev_assign_to_device is idempotent — it does if (associated(dst)) call dev_free(dst) first (FUNDAL/src/lib/fundal_dev_assign_agnostic.INC:331-398), which is why the startup repeat path is safe.

dev_alloc is not. fptr_dev is intent(out), pointer; the body unconditionally calls DEVALLOC and overwrites (fundal_dev_alloc_agnostic.INC). Any second dev_alloc on a still-associated pointer silently leaks the first buffer.

Consequence: the moment anything re-enters allocate_gpu — regrid, restart re-init, realm reuse — it leaks all 8 arrays including the rank-7 flxyz_c_gpu, with no diagnostic. The current safety rests entirely on that routine being called exactly once.

buf_5D_R8P and the db5/hb5 descriptors (adam_prism_fnl_object.F90:299-309) carry a matching staleness coupling: they are captured from nb/ni/nj/nk/nv at init. Nothing changes those post-init today, and nothing asserts that they cannot.

Proposed fix

  1. Add a destroy type-bound procedure to each of the 12 stateful FNL types, freeing every pointer it owns, associated()-guarded.
  2. Call them from finalize_forest (:4705-4713), per realm.
  3. Guard allocate_gpu's 8 dev_allocs with associated() checks — or convert them to dev_assign_to_device — so a future regrid cannot leak them silently.
  4. Consider a final on prism_fnl_object as a backstop, noting the usual caveat that final on a type containing pointer components does not fire recursively for pointer targets.

Deliberately not proposing FUNDAL-side changes to dev_alloc: making it free-safe would change documented semantics for all consumers. The guard belongs on the ADAM side.

Secondary defect found in the same sweep

src/app/prism/fnl/adam_prism_fnl_rk_pml_object.F90:50,71integer(I4P) :: ierr is declared in initialize and never assigned in that scope (the dev_assign_to_device calls do not take it; allocate_face_buffers has its own local at :148). Line :71 then reads it:

if (ierr /= 0_I4P) continue

An uninitialized read. Harmless in effect — the branch body is continue — but it is undefined behaviour and will trip any sanitizer or -Mchkptr-style build.


Companion issue: #33 (coil dev_assign_to_device size-dependent fault). The multi-realm leak described here is a plausible independent path to the same symptom.

Activity

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions