Fix str1d deallocation (#2161)

* Deallocate all the assigned memory in one go when modflow is finizalizing

* Fix for the deallocation of a string1d array

* Fix rebase mistake

* Remove obsolete comment

* Fix spelling

* Only apply workaround to older gfortran versions

* Rename all memory.f90 to .F90

* Add __GFORTRAN__

* Extend fix to also be applied to gfortran 13

* Restore developer deallocation check

msvs/mf6core.vfproj

@@ -549,7 +549,7 @@
 			<FileConfiguration Name="Release|Win32" ExcludedFromBuild="true"/></File>
 		<File RelativePath="..\src\Utilities\Matrix\SparseMatrix.f90"/></Filter>
 		<Filter Name="Memory">
-		<File RelativePath="..\src\Utilities\Memory\Memory.f90"/>
+		<File RelativePath="..\src\Utilities\Memory\Memory.F90"/>
 		<File RelativePath="..\src\Utilities\Memory\MemoryContainerIterator.f90"/>
 		<File RelativePath="..\src\Utilities\Memory\MemoryHelper.f90"/>
 		<File RelativePath="..\src\Utilities\Memory\MemoryManager.f90"/>

src/Utilities/Memory/Memory.f90 → src/Utilities/Memory/Memory.F90

@@ -29,7 +29,10 @@ module MemoryTypeModule
     logical(LGP), pointer :: logicalsclr => null() !< pointer to the logical
     integer(I4B), pointer :: intsclr => null() !< pointer to the integer
     real(DP), pointer :: dblsclr => null() !< pointer to the double
-    character(len=:), dimension(:), pointer, contiguous :: astr1d => null() !< pointer to the 1d character string array
+    ! The 1d character string array is handled differently than the other arrays due to a bug in gfortran 11.3 and 12.1.
+    ! Due to this bug the length of the string is not stored in the array descriptor. With a segmentation fault as a result
+    ! on deallocation.
+    class(*), dimension(:), pointer, contiguous :: astr1d => null() !< pointer to the 1d character string array
     integer(I4B), dimension(:), pointer, contiguous :: aint1d => null() !< pointer to 1d integer array
     integer(I4B), dimension(:, :), pointer, contiguous :: aint2d => null() !< pointer to 2d integer array
     integer(I4B), dimension(:, :, :), pointer, contiguous :: aint3d => null() !< pointer to 3d integer array
@@ -98,8 +101,12 @@ function mt_associated(this) result(al)
   end function mt_associated
 
   subroutine mt_deallocate(this)
+    use iso_c_binding, only: c_loc, c_ptr, c_null_ptr, c_f_pointer
     class(MemoryType) :: this
     integer(I4B) :: n
+    type(c_ptr) :: cptr
+
+    character(len=1), dimension(:), pointer :: astr1d
 
     if (associated(this%strsclr)) then
       if (this%master) deallocate (this%strsclr)
@@ -121,8 +128,30 @@ subroutine mt_deallocate(this)
       nullify (this%dblsclr)
     end if
 
+    ! Handle the dealloction of the 1d character string array differently due to a bug in gfortran 11.3, 12.1, 13.1 and 13.2.
+    ! Due to a bug in the gfortran compiler we can't use a deferred length character variable
+    ! https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106317
+    !
+    ! We use a c_ptr to cast the pointer to a string array with a length of 1. The actual length of the array is
+    ! computed by the actual length of the string multiplied by the array size.
+    ! So we go from the actual character(len=element_size), dimension(isize) to a character(len=1), dimension(isize*element_size).
+
     if (associated(this%astr1d)) then
+      select type (item => this%astr1d)
+      type is (character(*))
+        cptr = c_loc(item)
+      class default
+        cptr = c_null_ptr
+      end select
+
+      call c_f_pointer(cptr, astr1d, [this%isize * this%element_size])
+
+#if __GFORTRAN__  && ((__GNUC__ < 13) || (__GNUC__ == 13 && __GNUC_MINOR__ < 3))
+      if (this%master) deallocate (astr1d)
+#else
       if (this%master) deallocate (this%astr1d)
+#endif
+
       nullify (this%astr1d)
     end if
 

src/Utilities/Memory/MemoryManager.f90

@@ -484,9 +484,7 @@ subroutine allocate_str1d(astr1d, ilen, nrow, name, mem_path)
     allocate (mt)
     !
     ! -- set memory type
-    ! this does not work with gfortran 11.3 and 12.1
-    ! so we have to disable the pointing to astr1d
-    ! mt%astr1d => astr1d
+    mt%astr1d => astr1d
     mt%element_size = ilen
     mt%isize = isize
     mt%name = name
@@ -1162,6 +1160,7 @@ subroutine reallocate_str1d(astr, ilen, nrow, name, mem_path)
       deallocate (astrtemp)
       !
       ! -- reset memory manager values
+      mt%astr1d => astr
       mt%element_size = ilen
       mt%isize = isize
       mt%nrealloc = mt%nrealloc + 1
@@ -1596,7 +1595,12 @@ subroutine setptr_str1d(astr1d, name, mem_path)
     logical(LGP) :: found
     ! -- code
     call get_from_memorystore(name, mem_path, mt, found)
-    astr1d => mt%astr1d
+    select type (item => mt%astr1d)
+    type is (character(*))
+      astr1d => item
+    class default
+      astr1d => null()
+    end select
   end subroutine setptr_str1d
 
   !> @brief Set pointer to an array of CharacterStringType
@@ -1937,35 +1941,8 @@ subroutine deallocate_str1d(astr1d, name, mem_path)
     character(len=*), dimension(:), pointer, contiguous, intent(inout) :: astr1d !< array of strings
     character(len=*), optional, intent(in) :: name !< variable name
     character(len=*), optional, intent(in) :: mem_path !< path where variable is stored
-    ! -- local
-    type(MemoryType), pointer :: mt
-    logical(LGP) :: found
-    type(MemoryContainerIteratorType), allocatable :: itr
     ! -- code
-    !
-    found = .false.
-    if (present(name) .and. present(mem_path)) then
-      call get_from_memorystore(name, mem_path, mt, found)
-      nullify (mt%astr1d)
-    else
-      itr = memorystore%iterator()
-      do while (itr%has_next())
-        call itr%next()
-        mt => itr%value()
-        if (associated(mt%astr1d, astr1d)) then
-          found = .true.
-          exit
-        end if
-      end do
-    end if
-
-    if (found) then
-      if (mt%master) then
-        deallocate (astr1d)
-      else
-        nullify (astr1d)
-      end if
-    end if
+    return
 
   end subroutine deallocate_str1d
 
@@ -2461,7 +2438,6 @@ end function calc_virtual_mem
   subroutine mem_da()
     ! -- modules
     use VersionModule, only: IDEVELOPMODE
-    use InputOutputModule, only: UPCASE
     ! -- local
     class(MemoryType), pointer :: mt
     type(MemoryContainerIteratorType), allocatable :: itr
@@ -2471,6 +2447,7 @@ subroutine mem_da()
       call itr%next()
       mt => itr%value()
       call mt%mt_deallocate()
+      if (IDEVELOPMODE == 1) call mem_da_check(mt)
       deallocate (mt)
     end do
 
@@ -2480,6 +2457,39 @@ subroutine mem_da()
     end if
   end subroutine mem_da
 
+  subroutine mem_da_check(mt)
+    ! -- modules
+    use InputOutputModule, only: UPCASE
+    ! -- dummy
+    class(MemoryType), pointer :: mt
+    ! -- local
+    character(len=LINELENGTH) :: error_msg
+    character(len=LENVARNAME) :: ucname
+    !
+    ! -- check if memory has been deallocated
+    if (mt%mt_associated() .and. mt%element_size == -1) then
+      error_msg = trim(adjustl(mt%path))//' '// &
+                  trim(adjustl(mt%name))//' has invalid element size'
+      call store_error(trim(error_msg))
+    end if
+    !
+    ! -- check if memory has been deallocated
+    if (mt%mt_associated() .and. mt%isize > 0) then
+      error_msg = trim(adjustl(mt%path))//' '// &
+                  trim(adjustl(mt%name))//' not deallocated'
+      call store_error(trim(error_msg))
+    end if
+    !
+    ! -- check case of varname
+    ucname = mt%name
+    call UPCASE(ucname)
+    if (mt%name /= ucname) then
+      error_msg = trim(adjustl(mt%path))//' '// &
+                  trim(adjustl(mt%name))//' not upper case'
+      call store_error(trim(error_msg))
+    end if
+  end subroutine mem_da_check
+
   !> @brief Create a array with unique first components from all memory paths.
   !! Only the first component of the memory path is evaluated.
   !<

src/meson.build

@@ -349,7 +349,7 @@ modflow_sources = files(
     'Utilities' / 'Idm' / 'netcdf' / 'NCFileVars.f90',
     'Utilities' / 'Matrix' / 'MatrixBase.f90',
     'Utilities' / 'Matrix' / 'SparseMatrix.f90',
-    'Utilities' / 'Memory' / 'Memory.f90',
+    'Utilities' / 'Memory' / 'Memory.F90',
     'Utilities' / 'Memory' / 'MemoryHelper.f90',
     'Utilities' / 'Memory' / 'MemoryContainerIterator.f90',
     'Utilities' / 'Memory' / 'MemoryStore.f90',

utils/mf5to6/msvs/mf5to6.vfproj

@@ -82,7 +82,7 @@
 		<Filter Name="MF6">
 		<Filter Name="Utilities">
 		<Filter Name="Memory">
-		<File RelativePath="..\..\..\src\Utilities\Memory\Memory.f90"/>
+		<File RelativePath="..\..\..\src\Utilities\Memory\Memory.F90"/>
 		<File RelativePath="..\..\..\src\Utilities\Memory\MemoryContainerIterator.f90"/>
 		<File RelativePath="..\..\..\src\Utilities\Memory\MemoryHelper.f90"/>
 		<File RelativePath="..\..\..\src\Utilities\Memory\MemoryManager.f90"/>

utils/mf5to6/pymake/extrafiles.txt

@@ -1,4 +1,4 @@
-../../../src/Utilities/Memory/Memory.f90
+../../../src/Utilities/Memory/Memory.F90
 ../../../src/Utilities/Memory/MemoryContainerIterator.f90
 ../../../src/Utilities/Memory/MemoryHelper.f90
 ../../../src/Utilities/Memory/MemoryManager.f90