Fortran 90 function return pointer

Question

I saw this question:

Fortran dynamic objects

and the accepted answer made me question if I wrote the following function safely (without allowing a memory leak)

   function getValues3D(this) result(vals3D)
     implicit none
     type(allBCs),intent(in) :: this
     real(dpn),dimension(:,:,:),pointer :: vals3D
     integer,dimension(3) :: s
     if (this%TF3D) then
       s = shape(this%vals3D)
       if (associated(this%vals3D)) then
            stop "possible memory leak - p was associated"
       endif
       allocate(vals3D(s(1),s(2),s(3)))
       vals3D = this%vals3D
     else; call propertyNotAssigned('vals3D','getValues3D')
     endif
   end function

This warning shows up when I run my code, but shouldn't my this%vals3D be associated if it was previously (to this function) set? I'm currently running into memory errors, and they started showing up when I introduced a new module with this function in it.

Any help is greatly appreciated.

I think I wasn't specific enough. I would like to make the following class, and know how to implement the class, safely in terms of memory. That is:

   module vectorField_mod
   use constants_mod
   implicit none

   type vecField1D
     private
     real(dpn),dimension(:),pointer :: x
     logical :: TFx = .false.
   end type

   contains

   subroutine setX(this,x)
     implicit none
     type(vecField1D),intent(inout) :: this
     real(dpn),dimension(:),target :: x
     allocate(this%x(size(x)))
     this%x = x
     this%TFx = .true.
   end subroutine

   function getX(this) result(res)
     implicit none
     real(dpn),dimension(:),pointer :: res
     type(vecField1D),intent(in) :: this
     nullify(res)
     allocate(res(size(this%x)))
     if (this%TFx) then
       res = this%x
     endif
   end function

   end module

Where the following code tests this module

   program testVectorField
   use constants_mod
   use vectorField_mod
   implicit none

   integer,parameter :: Nx = 150
   real(dpn),parameter :: x_0 = 0.0
   real(dpn),parameter :: x_N = 1.0
   real(dpn),parameter :: dx = (x_N - x_0)/dble(Nx-1)
   real(dpn),dimension(Nx) :: x = (/(x_0+dble(i)*dx,i=0,Nx-1)/)
   real(dpn),dimension(Nx) :: f
   real(dpn),dimension(:),pointer :: fp
   type(vecField1D) :: f1
   integer :: i

   do i=1,Nx
    f(i) = sin(x(i))
   enddo

   do i=1,10**5
     call setX(f1,f) ! 
     f = getX(f1) ! Should I use this? 
     fp = getX(f1) ! Or this?
     fp => getX(f1) ! Or even this?
   enddo
   end program

Currently, I'm running on windows. When I CTR-ALT-DLT, and view performance, the "physical memory usage histery" increases with every loop iteration. This is why I assume that I have a memory leak.

So I would like to repose my question: Is this a memory leak? (The memory increases with every one of the above cases). If so, is there a way I avoid the memory leak while still using pointers? If not, then what is happening, should I be concerned and is there a way to reduce the severity of this behavior?

Sorry for the initial vague question. I hope this is more to the point.

Answer 1

Every time setX is called, any previously allocated memory for the x component of your type will be leaked. Since you call the function 10^5 times, you will waste 100000-1 copies. If you know that the size of this%x will never change, simply check to see if a previous call had already allocated the memory by checking to see if ASSOCIATED(this%x) is true. If it is, skip the allocation and move directly to the assignment statement. If the size does change, then you will first have to deallocate the old copy before allocating new space.

Two other minor comments on setX: The TARGET attribute of the dummy argument x appears superfluous since you never take a pointer of that argument. Second, the TFx component of your type also seems superfluous since you can instead check if x is allocated.

For the function getX, why not skip the allocation completely, and merely set res => this%x? Admittedly, this will return a direct reference to the underlying data, which maybe you want to avoid.

In your loop,

   do i=1,10**5
     call setX(f1,f) ! 
     f = getX(f1) ! Should I use this? 
     fp = getX(f1) ! Or this?
     fp => getX(f1) ! Or even this?
   enddo

fp => getX(f1) will allow you to obtain a pointer to the underlying x component of your type (if you adopt my change above). The other two use assignment operators and will copy data from the result of getX into either f, or (if it is previously allocated) fp. If fp is not allocated, the code will crash.

If you do not want to grant direct access to the underlying data, then I suggest that the return value of getX should be defined as an automatic array with the size determined by this%x. That is, you can write the function as

   function getX(this) result(res)
     implicit none
     type(vecField1D),intent(in) :: this
     real(dpn),dimension(size(this%x,1)) :: res
     res = this%x
   end function

Answer 2

Are you really restricted to Fortran 90? In Fortran 2003 you would use an allocatable function result for this. This is much safer. Using pointer function results, whether you have a memory leak with this code or not depends on how you reference the function, which you don't show. If you must return a pointer from a procedure, it is much safer to return it via a subroutine argument.

BUT...

This function is pointless. There's no point testing the association status of this%vals3D` after you've referenced it as the argument to SHAPE in the previous line. If the pointer component is disassocated (or has undefined pointer association status), then you are not permitted to reference it.

Further, if the pointer component is associated, all you do is call stop!

Perhaps you have transcribed the code to the question incorrectly?

---

If you simply delete the entire if construct starting with if (associated(this%vals3D))... then your code may make sense.

BUT...

• if this%TF3D is true, then this%vals3D must be associated.

• when you reference the function, you must use pointer assignment

  array_ptr => getValues3D(foo)
  !          ^
  !          |
  !          + this little character is very important.
  

  Forget that little character and you are using normal assignment. Syntactically valid, difficult to pick the difference when reading code and, in this case, potentially a source of memory corruption or leaks that might go undetected until the worst possible moment, in addition to the usual pitfalls of using pointers (e.g. you need to DEALLOCATE array_ptr before you reuse it or it goes out of scope). This is why functions returning pointer results are considered risky.

---

Your complete code shows several memory leaks. Every time you allocate something that is a POINTER - you need to pretty much guarantee that there will be a matching DEALLOCATE.

You have a loop in your test code. ALLOCATE gets called a lot - in both the setter and the getter. Where are the matching DEALLOCATE statements?