Deadlock on multithreaded usage due to UnlockSession always being called with callerHasLock=False

[toesus]

Description of issue

I encountered a deadlock situation when using niscope from multiple threads. Starting with version 1.4.3, the Session unlock has no effect and leads to other threads hanging forever waiting for the lock.

This is due to the use of a local variable caller_has_lock_ctype which always has the default value False.

nimi-python/generated/niscope/niscope/_library_interpreter.py

Line 642 in 28f446e

caller_has_lock_ctype = _visatype.ViBoolean() # case S220

System report

python -c "import niscope; niscope.print_diagnostic_information()" output (replace niscope with appropriate package name)

OS:
    Name:      Windows
    Version:   10.0.19041
    Bits:      64
Driver:
    Name:      NI-SCOPE
    Version:   21.0.0.49152
Module:
    Name:      niscope
    Version:   1.4.3
Python:
    Version:   3.7.9 (tags/v3.7.9:13c94747c7, Aug 17 2020, 18:58:18) [MSC v.1900 64 bit (AMD64)]
    Bits:      64
    Is_Venv:   False
    Installed Packages:
        zipp==3.7.0
        typing-extensions==4.1.1
        six==1.16.0
        setuptools==47.1.0
        pyvisa==1.11.3
        pyvisa-py==0.5.2
        pip==22.0.3
        numpy==1.21.5
        nitclk==1.4.3
        niscope==1.4.3
        nifgen==1.4.3
        nidmm==1.4.3
        nidaqmx==0.6.0
        importlib-metadata==4.11.0
        hightime==0.2.1

Steps to reproduce issue

import niscope
import threading


s = niscope.Session(resource_name="PXI2Slot7", reset_device=True)


def thread_function():

    print("Locking...")
    s.lock()
    print("Locked!")
    s.unlock()
    print("Unlocked!")



t1 = threading.Thread(target=thread_function)

t1.start()
t1.join()

t2 = threading.Thread(target=thread_function)
t2.start()
t2.join()

This script runs two threads sequentially. The first thread is started and does a lock/unlock.
The second thread then immediately hangs when trying to do the lock().
Output:

Locking...
Locked!
Unlocked!
Locking...

[marcoskirsch]

Oh boy.
We need a system-level test for this.

How do you plan to work around this in the short term? I'm not sure when we can have a fix in place and released.

[ni-jfitzger]

@toesus Impressive analysis.

From the C Documentation for niScope_UnlockSession:

callerHasLock	ViBoolean*	This parameter serves as a convenience; if you do not want to use this parameter, pass VI_NULL.Use this parameter in complex functions to keep track of whether you have obtained a lock and therefore need to unlock the session; pass the address of a local ViBoolean variable; in the declaration of the local variable, initialize it to VI_FALSE; pass the address of the same local variable to any other calls you make to niScope_LockSession or niScope_UnlockSession in the same function.

It seems the callerHasLock pointer should only be used if its first passed to niScope_LockSession, which isn't what's happening.

[toesus]

For the short term I simply went back to 1.4.1 (or 1.4.2 would be fine too I guess)
So for me I am not in a hurry, my program was running fine in production with 1.4.1, there was no need to update niscope.

I just wanted to report the issue after having spent some time debugging because it seems quite critical.
My actual program is similar to the example code I gave above, where the measurements are done in a thread which is separate from the main user-interface thread. So it is multithreaded, but the niscope access is only ever done from one thread at a time.
(Also I do not call the lock() manually, the issue appears when the niscope methods call lock() via that decorator thing.

Regarding the callerHasLock parameter, I also found the C library documentation confusing at first. The "convenience" they talk about seems to be for C programmers who do not care to remember how often they called lock() from the same thread?

From the Python side of things it seems certainly better to encourage people to use the context manager which guarantees the matching lock/unlocks. So I would not even bother to expose the callerHasLock parameter in the python API, and simply always pass None / NULL.

[Axxxeeel]

Hello,
I also ran into a multithreading issue using nidcpower 1.4.3 in a PyQt application where the measurements are done in a separate QThread, which would hang forever. Reverted to 1.4.1.

[marcoskirsch]

Also tracked by internal NI bug 2305832