Race condition when using module variables for library

[Olaf_Trygve_Berglihn]

The following code example is taken from Metcalf, Reid and Cohen, "Fortran 95/2003 explained", Oxford University Press, 2004.

My intuition tells me that parallel threads using the lib_code module can cause race conditions by using the same module variable 'struct'. If thread 1 starts calling initialize, but is not able to reach the data=c_loc(struct) statement before thread 2 has called initialize or add (with c_f_pointer), would this not lead to problems where the c_loc is taken on a different pointer target than intended?

module lib_code
use iso_c_binding
type :: pass
integer :: n
real, allocatable :: a(:)
: ! More stuff
end type(pass)
type(pass), pointer :: struct
contains
subroutine initialize(n, data) bind(c)
integer(c_int), value :: n
type(c_ptr), intent(out) :: data
allocate(struct)
struct%n = n
allocate(struct%a(n))
data = c_loc(struct)
end subroutine initialize
subroutine add(data, ...) bind(c)
type(c_ptr), intent(in) :: data
:
call c_f_pointer(data, struct)
:
end subroutine add
end module lib_code

[jimdempseyatthecove]

If you have a race condition then you have a coding error on your part.

your subroutine initialize must be called (and returned) for each object it is intended to allocate _prior_ to your threads manipulating that (those) object(s). This is your responsibility.

When allocation is made inside a parallel region then you must protect the access with proper statements:

!$omp parallel
...
!$omp single
call initialize(...
!$omp end single
!$omp barrier
... use pointer here
!$omp end parallel

Generally you perform your allocations and initializations in your serial sections of code. However, yo may have the odd occasion to perform the allocation inside a parallel region and in there you must take the responsibility to avoid race conditions. (also handle allocation failures)

Jim Dempsey

[Olaf_Trygve_Berglihn]

So what you are saying, Jim, is that there is no alternative in fortran to having the module variable "struct" in the module scope, and thus that the initialize subroutine cannot be made such that a race condition is avoided?

Point is, that when I read the example in "fortran 95/2003 explained", I immediately reacted to the possible race condition when calling the module subroutines in parallel. It can be avoided in the subroutine "add" by introducing a local "struct" variable, but I have not figured out a way of doing it in the "initialize" subroutine without the "struct" variable possibly going out of scope and thus causing a segfault.

My general opinion is that code should be written such that the race condition is avoided in the library code itself, and not leave this responsibility to the caller ... as long as the language premits.

So to rephrase the question: is there another way of doing it, without opening for a race condition?

UPDATE:
I asked the authors and got a reply from Reid. He agrees there is a race condition and that a lock is required in "initialize". So the answer to my question is so far that there is no other way than that using a shared (module) variable for initialization to avoid deallocation of the structure on exit of "initialize" when using bind(c).

[Olaf_Trygve_Berglihn]

Actually, there seems to be a solution: nullify()

Reading about the nullify() function it seems this function will prevent the locally allocated "struct" from being deallocated on exit of "initialize". The book example can be modified to the code below, and it seems to be working ok.

module lib_code
    use iso_c_binding
    type  :: pass
        integer :: n
          :  ! More stuff
    end type(pass)
!    type(pass), pointer :: struct ! Module variable not required.
contains
    subroutine initialize(n, data)  bind(c)
        type(pass), pointer :: struct ! Added local struct.
        integer(c_int), value :: n
        type(c_ptr), intent(out) :: data
        allocate(struct)
        struct%n = n
        allocate(struct%a(n))
        data = c_loc(struct)
        nullify(struct) ! Added to avoid deallocation on exit.
    end subroutine initialize
    subroutine add(data, ...) bind(c)
        type(pass), pointer :: struct ! Added local struct.
        type(c_ptr), intent(in) :: data
            :
        call c_f_pointer(data, struct)
            :
    end subroutine add
end module lib_code

[jimdempseyatthecove]

The following will work

[bash]module lib_code
    use iso_c_binding
    type  :: pass
        integer :: n
        real, allocatable :: a(:)
          :  ! More stuff
    end type(pass)
contains
    subroutine initialize(n, data)  bind(c)
        integer(c_int), value :: n
        type(c_ptr), intent(out) :: data
        type(pass), pointer :: struct
        integer :: err
        data = 0 ! should have been null on entry
        err = 0
        allocate(struct, stat=err)
        if(err .ne. 0) return
        struct%n = n
        allocate(struct%a(n), stat=err)
        if(err .ne. 0) then
           deallocate(struct)
           return
        endif
        data = c_loc(struct)
    end subroutine initialize
    subroutine add(data, ...) bind(c)
        type(c_ptr), intent(in) :: data
        type(pass), pointer :: struct
            :
        call c_f_pointer(data, struct)
            :
    end subroutine add
    subroutine destroy(data)  bind(c)
        type(c_ptr), intent(inout) :: data
        type(pass), pointer :: struct
        
        if(data .ne. 0) then
          call c_f_pointer(data, struct)
          if(allocated(struct%a)) then
            deallocated(struct%a)
          endif
          deallocate(struct)
          data = 0
        endif
    end subroutine initialize
end module lib_code

----------------------

// C++
   pass_t*  p_pass = NULL;  // NULL until OK
   ...
   initialize(n, p_pass); // or could write as function
   if(!p_pass) abort();
   ...

// Note, your responsibility as to:
// lifetime of object pointed to by p_pass
// and only one thread to delete object pointed to by p_pass
[/bash]

[Olaf_Trygve_Berglihn]

Jim: No, this code fails on the latest 11.1 due to "struct" being deallocated on exit of "initialize", leading to "data" pointing to possible random memory. One could argue that the c_loc should inhibit the compiler from deallocating "struct", but I think in this case that an explicit nullify(struct) on exit is ok.

[Steven_L_Intel1]

As struct is a pointer, the compiler does not deallocate it unless you do so explicitly. Using C_LOC doesn't change that.

If it were an ALLOCATABLE and not given SAVE, then it would be automatically deallocated on exit, but it isn't.

Jim's code has a problem in that it assigns 0 to a variable of type C_PTR. Can't do that - C_PTR is a derived type with private components. You can assign it the value C_NULL_PTR.

[jimdempseyatthecove]

Quoting Steve Lionel (Intel)

Jim's code has a problem in that it assigns 0 to a variable of type C_PTR. Can't do that - C_PTR is a derived type with private components. You can assign it the value C_NULL_PTR.

Thanks for pointing this out. A relatively easy fix to the code.

Jim