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

Memory error in recursive type initialization #132

Open
bernardopacini opened this issue Nov 6, 2020 · 7 comments
Open

Memory error in recursive type initialization #132

bernardopacini opened this issue Nov 6, 2020 · 7 comments

Comments

@bernardopacini
Copy link
Contributor

bernardopacini commented Nov 6, 2020

Following up on the issue encountered with PR #131.

I believe the issue is specific to recursive types, so below is a simpler example (based on the recursive type array example) that shows the same problem.

Fortran:

module mod_recursive_type_array
  implicit none
  type t_node
    type(t_node),dimension(:),allocatable :: node
  end type t_node
  
contains

  subroutine allocate_node(root,N_node)
    type(t_node),intent(out) :: root
    integer,intent(in) :: N_node

    allocate(root%node(N_node))

  end subroutine allocate_node

  subroutine deallocate_node(root)
    type(t_node) :: root

    if (allocated(root%node)) deallocate(root%node)

  end subroutine
end module mod_recursive_type_array

Checked with the Python script:

import unittest
import numpy as np

from recursive_type_array import Mod_Recursive_Type_Array as recursive_type_array

class Test_recursive_type_array(unittest.TestCase):
    def setUp(self):
        self.N_node = 3

        self.root = recursive_type_array.t_node()

        self.root = recursive_type_array.allocate_node(self.N_node)
        self.root.node[0] = recursive_type_array.allocate_node(self.N_node)

    def tearDown(self):
        recursive_type_array.deallocate_node(self.root)

    def test_allocate(self):
        self.assertEqual(
            len(self.root.node),
            self.N_node
        )

    def test_deallocate(self):
        recursive_type_array.deallocate_node(self.root)
        self.assertEqual(
            len(self.root.node),
            0
        )

if __name__ == "__main__":
    unittest.main()

WIth this update, I run into the following error caused by the line self.root.node[0] = recursive_type_array.allocate_node(self.N_node):

Python(8902,0x1095b8dc0) malloc: *** error for object 0x3000000000000000: pointer being freed was not allocated
Python(8902,0x1095b8dc0) malloc: *** set a breakpoint in malloc_error_break to debug
[1]    8902 abort      python3 tests.py

When I use the previous commit (c4e8ea9) instead, the tests complete without an issue.

Thanks again!

@bernardopacini
Copy link
Contributor Author

Hi again,

Changing the subroutine to be intent(inout) rather than intent(out) solves this issue, but I am not sure if this is a complete fix.

One could argue that for this specific case the subroutine should be inout as the node object already exists, but this may not be true in all cases.

Thanks

@jameskermode
Copy link
Owner

This is a proposed fix, as explained in the discussion at #131

diff --git a/f90wrap/pywrapgen.py b/f90wrap/pywrapgen.py
index 3657ad0..4b95994 100644
--- a/f90wrap/pywrapgen.py
+++ b/f90wrap/pywrapgen.py
@@ -380,6 +380,7 @@ except ValueError:
         self.write('if self._alloc:')
         self.indent()
         self.write('%(mod_name)s.%(prefix)s%(func_name)s(%(f90_arg_names)s)' % dct)
+        self.write('self._alloc = False')
         self.dedent()
         self.dedent()
         self.write()

would you be willing to test this?

@bernardopacini
Copy link
Contributor Author

Hi,

Thank you for looking into this. I tried out the fix but I still run into the same segmentation fault. I am not sure if there is an issue with calling deallocate as the error occurs at the below line even when I remove any deallocate call from Python.

self.root.node[0] = recursive_type_array.allocate_node(self.N_node)

What is interesting is that calling the function once is okay and results in a new object self.root. It is the following call for adding in the node self.root.node[0] that causes the failure. You can also generate a totally new object self.root2 without issue.

Thank you again!

@jameskermode
Copy link
Owner

Ok, thanks for testing. I’ll try to run it myself soon. I think it it still related to the automated destructor, but must be slightly more subtle.

@bernardopacini
Copy link
Contributor Author

Got it -- thank you! Please let me know how I can help.

@jameskermode
Copy link
Owner

Quick update: I've reproduced the issue, and hope to find a fix soon.

@jameskermode
Copy link
Owner

Renaming the allocate and deallocate Fortran routines to node_allocate and node_deallocate respectively gets me a little further, as they are then recognised as the constructor and destructor for t_node objects and called automatically. There’s still a double free for the node stored within the array slot, which I’ll look into further.

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