DESI-UR / VAST

Void Analysis Software Toolkit
https://vast.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
9 stars 8 forks source link

Potential fix for the BufferError causing the GitHub Unit Tests to fail #85

Closed QuiteAFoxtrot closed 2 years ago

QuiteAFoxtrot commented 2 years ago

Based on Hernon's findings here: https://github.com/hbrincon/VAST/runs/8043325424?check_suite_focus=true&fbclid=IwAR22fY_NHNEZBAvM0lsDVXXwt1d9y_senM1w5vp1kkbKx6lZW3IQ7CWooxw

Issue with unit tests failing for Py3.8 and Py3.9 appear to be due Python itself not allowing us to call the close() method on a mem-map we have created (a sort of unnecessary safety check Python is imposing on us...) because we are pointing to that map with one of the class attributes, so even though we're going to immediately re-point that class attribute to the new memory we're about to use, for a brief moment Python thinks we're going to leave it pointing to bad memory and is throwing this BufferError on us.

This PR contains a fix for this - unfortunately I could not implement the fixes located here: https://stackoverflow.com/questions/53339931/properly-discarding-ctypes-pointers-to-mmap-memory-in-python or here: https://github.com/ercius/openNCEM/issues/39

1). because the Cython memoryview object does not actually have a "release()" method 2). The object containing the pointer is not a regular python object, so calling "del curr_arr" did not work either 3). I also could not explicitly call dealloc (though potentially could revisit this maybe if we 'cimport' the actual memoryview class?)

So to circumvent this issue, I'm implicitly relying on the internal cython memoryview machinery and hoping the cython guys got it right - instead of calling release() or close() or 'del' on the memoryview containing the pointer, I am now creating a very small dummy array, pointing the offending memoryview to the dummy array, THEN closing the memmap and re-pointing the memoryview to that memory.