Issue releasing device resources from 0.3.1 to 0.4.2

[ericreichwein]

I have a function:

def func():
     daq = NIDAQ('Dev1')
     task = Task(daq.ao0, daq.ai0)
     task.set_timing(duration='1s', fsamp='1000Hz')
     task.run(data) # data is a dict

The first time I run this function it works as expected. The second time I run this function it causes error stating that device is already in use. I only get this when using 0.4.2, but when reverting back to 0.3.1 this goes away and works great. I haven't tried other versions, but will soon.

[natezb]

Do you have a full code example that reproduces this issue? I ask because I think this issue should only happen daq somehow persists between function calls.

In the 0.4 release, Instruments now check if they're already open, and will raise the error message that you've encountered if you try to open them again. For many drivers this is a useful check, as certain libraries can get in weird states if you try to open devices multiple times. In the case of NI-DAQmx, a DAQ never really gets "opened", so this isn't so much of an issue, and we could perhaps relax the check.

However, in the code you've provided here, it looks like daq should go out of scope and be cleaned up when the function returns. Either there's something broken with this cleanup process, or a reference to the DAQ is somehow leaking out of the function body.

The simplest solution in the near-term would be to create daq only once, outside of the function, and reuse it on every call.

[ericreichwein]

Thanks for quick reply! Probably should mention it is a large PyQt5 class. func() gets called by a button with signal/slot. The only way to get rid of the reference to daq is destroy the ipython session and restart, which releases the resources and I can connect again. I don't think it is on my end because it works great with instrumental-lib 0.3.1, but fails with all 0.4.x versions. Here is a full example of that function except my dictionary code which is before NIDAQ() and doesn't use/need anything than standard python:

class MyApp(QtWidgets.QMainWindow, Ui_MainWindow):
    def __init__(self):
        QtWidgets.QMainWindow.__init__(self)
        Ui_MainWindow.__init__(self)
        self.setupUi(self)
        self.button.clicked.connect(self.func)
        #init code here

    def func(self):
         #a bunch of code that generates data dictionary for task.run()

         daq = NIDAQ('Dev1')
         task = Task(daq.ao0, daq.ai0)
         task.set_timing(duration='1s', fsamp='1000Hz')
         task.reserve()
         task.run(data) # data is a dict
         task.unreserve()

if __name__ == "__main__":
    #app = QtWidgets.QApplication(sys.argv)
    if not QtWidgets.QApplication.instance():   
        app = QtWidgets.QApplication(sys.argv)
    else:
        app = QtWidgets.QApplication.instance() 
    window = MyApp()
    sys.exit(app.exec_())

[natezb]

I just pushed a change (ef5de3ab37c2c84c24e625f5244fb7ba70957a5a) that should hopefully fix this for you.

The problem was that Task has a __del__ method, which prevents CPython from cleaning up the circular references. The task holds references to some channels, which hold references to their respective DAQ(s). This prevents a DAQ object from being cleaned up, even when it goes out of scope. This wasn't such a problem in 0.3, since we didn't check if an instrument already existed before trying to open it, we'd just blindly reopen it, as I described above.

With that said, I'd still generally recommend only opening your instrument once and passing it around if you're going to reuse it. For example, opening it in your app's __init__ method and saving it as self.daq for use within your func method.

Thanks for the report, and let me know if that change fixed things for you.

[natezb]

I'm going to close this since the change fixed the cleanup problem for me. Feel free to reopen this if you're still seeing the issue.

[jondoesntgit]

I'm running into this issue as well:

from instrumental import instrument
daq = instrument('daq')
daq.close()
daq2 = instrument('daq')

InstrumentExistsError: Device already open

[jondoesntgit]

Kludgy work-around

from instrumental.drivers import Instrument
Instrument._allow_sharing = True

[jondoesntgit]

Finding that this also is occurring for other instruments. They are not released when I use the close method. They are also not released when I use the del operator.

[natezb]

As for your DAQ example, that error is the expected result, since the daq object still exists. The current error message is technically incorrect though, since the daq exists but isn't open. As of now, there's no generic notion of open vs closed for Instruments, and defining one might be tricky.

As for your other instruments, I can't exactly say what's happening without seeing the exact code being run, but I suspect there's at least one reference to the instrument(s), which keeps them from being cleaned up (del will only remove one reference, there may be more). It's possible that one or more of the drivers are broken such that they hold these extra references.

With all that said, this "feature" seems to have introduced more pain than it's worth, and probably unwarranted pain in most cases, so I'm open to relaxing the restriction. I could imagine supporting a flag upon instrument creation to allow different options:

1. Disallow opening an existing instrument (the current behavior, have to be very careful to keep track of all references)
2. If the instrument already has an instance, return it. Otherwise open it.
3. Create a new instance of the object, even if one exists. This is potentially dangerous, as the objects' states won't be synchronized and you could get yourself into all kinds of trouble. Probably still worth having this as an escape hatch though, as long as we warn of the danger.

Let me know what you think of this. Also, there may be reasonable "modes" that I'm missing here.

[jondoesntgit]

Perhaps there is a way for a process to obtain a lock, similar to how threads work.

_{Sent with GitHawk}

[natezb]

Please check out 4831d80149aaa10c03271fb5e14ee88c7cd9fbe2, which implements this feature for instrument() (and changes the default behavior) and tell me what you think.

[jondoesntgit]

This looks great to me! Thanks