Mutex for serialIO

[Oliver Schwanke]

the serialIO dataIO breaks the windows usual habit of one user per IO thing, by letting the dataIO handle be passed around between different classes, which is a veryvery useful and good thing. But to prevent race conditions between different classes, a mutex that can be obtained (ofc in the manner that the user has to take care of looking if the mutex is locked on use) seems useful

so my proposal is to implement two more functions, like lock and unlock and optionally one more bit like force_locked_operation(if set, a warning is returned when an unlocked write/read operation is performed, could also be tied to the debug flag)

Function should be like class foo-> claims lock(timeout) class foo writes class foo reads class foo-> unlocks merely like the QMutex functionality.

this function should be optional to use(that's why the force_locked_operation bit). It's mostly interesting for multipurpose/multifunctioning uControllerprojects where it's not possible to having a composite device stack on the device side, but having different device abstractions on PC/hostside is useful. But the implied overhead is too much for simple projects.

[M. Gronle]

I am sorry, that I didn’t see this issue until today, since I didn’t get notifications about new issues in the plugins repository

After I have read this proposal, I had to think about this for a couple of minutes. This means, that I find it very hard to decide if this goes into the ‘right’ direction. With ‘right’ direction I mean, that we should make sure that itom follows its major objectives: Of course, we should develop itom such that it can be used for all desired purposes. On the other hand, one major goal of itom is, to implement as far as possible features which are equal for all type of similar devices.

For the current purpose this means, that such a mutex should not only be implemented in the specific serialIO device, but in all I/O devices or more general for all kind of plugins. Else we would introduce one ‘solitaire’ solution for serialIO and then we have to think about the same for libUSB, modbus…
The inital idea of itom’s plugins where, that every dataIO and grabber plugin lives in its own thread, such that all methods should only be called by invoking them and not by directly calling them. If one would not directly call the getVal or setVal method of serialIO but invoke them, a thread-safe implementation would be guaranteed without any additional mutex. Currently, we only used directly calls to getVal or setVal or cameras or I/O devices if it is assured that only one caller is using these plugins. Now, there seems to be a problem where multiple callers should use the same serialIO instance in parallel. For this, I would find different solutions:

1. Invoking serialIO::getVal / serialIO::setVal. Pro: already available, fits to the desired architecture of itom, thread-safe. Contra: a little bit slower than a direct call, no protection for a series of calls available
2. Implementing the propsed feature for serialIO only (with the chance to extent this in the same way to other plugins): Pro: easy solution, fast, mutex protection for a series of calls available; Contra: no generalized approach for all kind of plugins; the ways how to call plugins are extended to a third possibility (direct, direct with mutex, invoke), one has to think if every method of a plugin should be protected by this mutex or only one method…
3. Finding a general solution for all plugins (somehow added in the base classes).
4. If the problem only occurs in very rare cases, it would also be possible to add a mutex as dynamic property to the serialIO instance (since derived from QObject). Then every caller could get this mutex and try to acquire it before calling the direct method. Then the thread-safe mechanism is fully implemented at caller-level and the plugin has not to be changed.

Maybe it would really be worth to check if the ready-to-use solution 1 would work (including how much slower this would be). If not, the should try to find an easy solution which could be applied to all plugins without major modifications. The reason for better finding such a generalized solution is, that the maintainance of the high number of plugins is already hard now. If we would start inserting such special gimmicks in single plugins, we have to pay attention that we don’t lose the overview and the maintainance possibility with our limited resources.

[M. Gronle]

One further remark to point 3 from my comment above:

It would also be possible to implement a small wrapper class for dataIO and another one for actuator instances. This wrapper could have the same methods than its wrapped interface (dataIO / actuator) and could get the real instance in its constructor. Then it contains the mutex as member. If you can a method like getVal or setVal over this wrapper, it tries to acquire the mutex and if possible, calls the real method getVal of its wrapped instance. This could be applied to all kind of plugins without modifications and the overhead is simply one additional function call.

A similar helper or wrapper method already exists to provide a simple interface for easily accessing cameras using the thread-safe invoke approach without the need to handle the async calls and consider possible timeouts… (see classes in pluginThreadCtrl.h in itomCommonQt)

[Oliver Schwanke]

Hi Marc,

This is not an additional threadsafety step, but some sort of processcontrol, which is needed when multiple read/writes need to be performed consecutively.

This means i need a way to claim/release the lock separate from set/getVal functions.

And being transmission protocols libmodbus, serialIO and libusb are more low level dataIO things than a complete camera plugin, which i think won’t need this kind of protection.

As i mentioned this is useful for microcontroller Projects, as in “i have an Arduino here and want to read out a lightmeter(own IF), do some computing and set a motor according to it(own IF)” which you could also port to two distinct microcontrollers, so you have two different devices connected to your PC. Of course the lock could also be ported to this microcontroller, but implies both IFs to implement this the same way.

This is not useful for all other purposes as all the other dataIO things. The most used USBDevice on these types of projects is USBCDC(all the 8 bit arduinos have these) as it being very general purpose. And rarely the USBBulk device class.(CDC connects as COMport(usbser.sys) on windows and(if done right) bulk may connect as USBdevice(winusb.sys) on windows(7+)).

‌

I have never dived into modbus, so i cannot say anything about that(nor did i compile the plugin). But doesn’t it use adresses internally(TCP/IP)? So theres only one IF connected to it? This would be sufficient processcontrol and make the lock unnecessary IMHO.

So is your proposed solution #4 available on the python console level? or does it need direct qt-interaction?

[Oliver Schwanke]

And being transmission protocols libmodbus, serialIO and libusb are more low level dataIO things than a complete camera plugin

‌

whereas the PGrey Cam we use here supports UART transmision to connected uCs, which might need this type of protection also…

[M. Gronle]

My idea is:

1. Add a QMutex as member of ito::AddInBase (goes into the d_ptr, such that binary compatibility is not destroyed)
2. ito::AddInBase get a new member method QMutex* getUserMutex() that returns the pointer to this mutex. Using this pointer C++ plugins can lock, tryLock, unlock etc. this mutex (also binary compatible)
3. The python classes itom.dataIO and itom.actuator will get 2-3 methods like lockUserMutex(timeout: int = -1) and unlockUserMutex. Lock will block python until the mutex could be locked or the timeout expired (-1: inifinite).

Using the python and c-interface uninterruptible calls to getVal, setVal etc. could be optionally protected by this mutex. However it is the responsibility of the programmer / user to properly use this user-defined mutex.

Would this idea solve the problem in a simple, but generalized approach?

‌

[M. Gronle]

Fixed in the pull request 56 of the itom project (https://bitbucket.org/itom/itom/pull-requests/56)