Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Native WrappedDataStore can leak device memory #234

Closed
l90lpa opened this issue Oct 25, 2024 · 1 comment
Closed

Native WrappedDataStore can leak device memory #234

l90lpa opened this issue Oct 25, 2024 · 1 comment

Comments

@l90lpa
Copy link
Contributor

l90lpa commented Oct 25, 2024

What happened?

First, I'm assuming that the intended semantics of atlas::array::native::WrappedDataStore are that it has a non-owning handle to host memory, and has an owning handle to device memory (if allocated).

In the class's current form, the class only has an implicit default destructor. Therefore, if there is allocated device memory on a class instance at the time of its destruction this device memory is leaked because there is no deallocation performed.

A fix might be to add:

~WrappedDataStore() {
    deallocateDevice();
}

to the class.

What are the steps to reproduce the bug?

Build Atlas with CUDA support enabled and OpenACC support disabled, then do something like:

#include "atlas/array/native/NativeDataStore.h"
...
int data[10];
{
    atlas::array::native::WrappedDataStore<int> ds(data, 10);
    ds.allocateDevice();
} // <-- memory leak here after ~WrappedDataStore()

Version

develop

Platform (OS and architecture)

Ubuntu 22.04

Relevant log output

No response

Accompanying data

No response

Organisation

JCSDA

@wdeconinck
Copy link
Member

Hi yes you're correct! I will fix this immediately. Thanks for reporting.

wdeconinck added a commit that referenced this issue Nov 18, 2024
* release/0.40.0: (40 commits)
  Update changelog
  Version 0.40.0
  Add hicLaunchHostFunc for async host functions
  Add test for various use cases of StructuredInterpolation2D with limiter
  Implement StructureInterpolation2D for 3-dimensional fields with extra variables)
  Fix application of limiter for StructuredInterpolation2D
  Protect atlas_test_array_view_variant to eckit v 1.27.0
  Move example-fortran inside atlas_HAVE_DOCS (#235)
  Fix possible device-memory leak in WrappedDataStore (fixes #234)
  add device data on FieldSet
  Add test for fieldset device operators
  Add fieldset device operators
  Update fctest_field_gpu to test acc deviceptr
  Update test_field_acc to test acc deviceptr
  Add device_data Fortran access to Field
  Refactored spherical vector interpolation method. (#227)
  Refactored (un)pack_vector_fields. (#226)
  Integrate `pack_vector_fields` into `SphericalVector` Interpolation method. (#224)
  Fixed ordering of fixup_halos and halo_exhange in StructuredColumns.cc (#223)
  Tidied headers and error message. (#231)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants