fix double close of GPIB interface and controller in GpibSession #171
Conversation
GpibSession uses Gpib.Gpib objects to manage interfaces to the instrument and GPIB board. The Gpib.Gpib objects do not expose a proper close() method. Rather, they rely on __del__() being called which is a potentially ugly implementation detail of CPython. As long as we use Gpib.Gpib instead of module gpib directly, we should avoid double-close and only make sure that GpibSession.close() deletes self.interface and self.controller.
|
Indeed this looks like a bad idea. At least the |
|
I will merge sometimes this week since this is no matter what the sensible solution for now. |
|
I will release gpib_ctypes that has close() as you suggest. However, I would not rule out using linux-gpib.Gpib. I've pushed a second commit which patches in a close() method. I somehow feel it is important to support the original Python GPIB bindings since it has the largest user base. In the mean time I can ping the linux-gpib project and see if they would be willing to add close(). |
tivek
force-pushed
the
fix_double_close
branch
from
5968788
to
a725016
Compare
|
linux-gpib has merged a patch that introduces Gpib.close() like gpib_ctypes 0.2.0. Until it gets released, and to support users of older linux-gpib releases, I propose to keep the monkeypatching code from 5968788. |
|
Sounds good. I agree that keeping the monkeypatch for now makes sense. Is this ready for merging ? |
|
Ready for merging. Tested with linux-gpib r1767 (before the accepted patch), linux-gpib trunk as well as gpib_ctypes 0.2.0. |
|
bors r+ |
171: fix double close of GPIB interface and controller in GpibSession r=MatthieuDartiailh a=tivek
Fix a double close of GPIB connections which usually produces uncaught exceptions.
To reproduce the bug on pyvisa-py master:
```python
$ ipython
Python 3.7.0 (default, Oct 9 2018, 10:31:47)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.1.1 -- An enhanced Interactive Python. Type '?' for help.
In [1]: import visa
In [2]: rm = visa.ResourceManager('@py')
In [3]: tek = rm.open_resource('GPIB0::30::INSTR')
In [4]: tek.query('*IDN?')
Out[4]: 'TEKTRONIX,TDS 3014C,0,CF:91.1CT FV:v4.05 TDS3GV:v1.00 TDS3FFT:v1.00 TDS3TRG:v1.00\n'
In [5]: quit()
libgpib: invalid descriptor
libgpib: invalid descriptor
Exception ignored in: <function Gpib.__del__ at 0x7f985e1996a8>
Traceback (most recent call last):
File "/home/tivek/projects/linux-gpib-code/linux-gpib-user/language/python/Gpib.py", line 32, in __del__
gpib.GpibError: close() failed: One or more arguments to the function call were invalid.
```
Explanation and fix:
GpibSession uses two Gpib.Gpib objects (linux-gpib project) to manage connections to the instrument and GPIB board. These clean up on their own in `__del__()` and we should not manually close their handles.
Long-term, `__del__()` should not be used for reliable finalization since it is a potentially ugly implementation detail of CPython's GC. Sadly, Gpib.Gpib does not expose a better interface for cleanup. At some point we should either patch/fork Gpib.Gpib or switch to classic gpib function calls with integer handles.
Co-authored-by: Tomislav Ivek <tomislav.ivek@gmail.com>
Build succeeded |
|
I forgot to ask and I am going to fix it right away but in the future please update the release notes. |
|
Sorry about the release notes, will do next time. |
|
It is no big deal. I am just trying to keep the release notes up to date so that the release process is a bit more straightforward. |
Fix a double close of GPIB connections which usually produces uncaught exceptions.
To reproduce the bug on pyvisa-py master:
Explanation and fix:
GpibSession uses two Gpib.Gpib objects (linux-gpib project) to manage connections to the instrument and GPIB board. These clean up on their own in
__del__()and we should not manually close their handles.Long-term,
__del__()should not be used for reliable finalization since it is a potentially ugly implementation detail of CPython's GC. Sadly, Gpib.Gpib does not expose a better interface for cleanup. At some point we should either patch/fork Gpib.Gpib or switch to classic gpib function calls with integer handles.