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

tests failures against Pytest 8.2.0 #4128

Closed
stanislavlevin opened this issue May 30, 2024 · 4 comments · Fixed by #4129
Closed

tests failures against Pytest 8.2.0 #4128

stanislavlevin opened this issue May 30, 2024 · 4 comments · Fixed by #4129
Labels
Milestone

Comments

@stanislavlevin
Copy link

As of Pytest 8.2.0 (bisected to pytest-dev/pytest#12096) silx' tests suite fails, for example:

(.run_venv) [builder@localhost python3-module-silx-2.1.0]$ python run_tests.py --installed -ra -Wignore --low-mem .run_venv/lib/python3/site-packages/silx/io/test/test_specfilewrapper.py::TestSpecfilewrapper::test_number_of_scans
================================ test session starts =================================
platform linux -- Python 3.12.2, pytest-8.2.0.dev25+g0dc036035, pluggy-1.5.0
rootdir: /usr/src/RPM/BUILD/python3-module-silx-2.1.0/.run_venv/lib/python3/site-packages/silx
configfile: ../../../../../pytest.ini
plugins: mock-3.14.0, xvfb-3.0.0
collected 1 item                                                                     

.run_venv/lib/python3/site-packages/silx/io/test/test_specfilewrapper.py .     [100%]

================================= 1 passed in 0.04s ==================================
AttributeError: 'Specfile' object has no attribute 'close'
Exception ignored in: 'silx.io.specfile.SpecFile.__dealloc__'
AttributeError: 'Specfile' object has no attribute 'close

Not sure if this is silx issue and anything can be done on its side.

@vallsv
Copy link
Contributor

vallsv commented May 30, 2024

Sounds like it's a call during the termination phase of python.

At this stage, access to object/module/package attributes are unpredictable. Because this objects was potentiall already released. That's a problem on it's own with the Specfile object. I mean __dealloc__ should maybe be protected against that.

But it also means this tests from test_specfilewrapper also do not clean up properly the resources. The resources should not be alive after the tests.

@stanislavlevin
Copy link
Author

This is what docs say about __dealloc__:
https://cython.readthedocs.io/en/latest/src/userguide/special_methods.html#finalization-methods-dealloc-and-del

You need to be careful what you do in a dealloc() method. By the time your dealloc() method is called, the object may already have been partially destroyed and may not be in a valid state as far as Python is concerned, so you should avoid invoking any Python operations which might touch the object. In particular, don’t call any other methods of the object or do anything which might cause the object to be resurrected. It’s best if you stick to just deallocating C data.

@t20100 t20100 added the bug label May 31, 2024
@t20100 t20100 added this to the Next release milestone May 31, 2024
@t20100
Copy link
Member

t20100 commented May 31, 2024

The SpecFile.__dealloc__ code is calling a method, so clearly out of the specification:

def __dealloc__(self):
"""Destructor: Calls SfClose(self.handle)"""
self.close()
def close(self):
"""Close the file descriptor"""
# handle is NULL if SfOpen failed
if self.handle:
if specfile_wrapper.SfClose(self.handle):
_logger.warning("Error while closing SpecFile")
self.handle = NULL

Calling close from __del__ instead of __dealloc__ should fix this.

https://cython.readthedocs.io/en/latest/src/userguide/special_methods.html#finalization-methods-dealloc-and-del

Python 3.4 made it possible for extension types to safely define finalizers for objects. When running a Cython module on Python 3.4 and higher you can add a __del__() method to extension types in order to perform Python cleanup operations. When the __del__() is called the object is still in a valid state (unlike in the case of __dealloc__()), permitting the use of Python operations on its class members. On Python <3.4 __del__() will not be called.

@t20100
Copy link
Member

t20100 commented May 31, 2024

Thanks for the bug report!

PR #4129 should fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants