mattjlewis / diozero

Java Device I/O library that is portable across Single Board Computers and microcontrollers. Tested with Raspberry Pi, Odroid C2, BeagleBone Black, Next Thing CHIP, Asus Tinker Board and Arduinos / Pico. Supports GPIO, I2C, SPI as well as Serial communication. Also known to work with Udoo Quad.
https://www.diozero.com
MIT License
261 stars 59 forks source link

I2C devices are not thread-safe #169

Closed EAGrahamJr closed 12 months ago

EAGrahamJr commented 1 year ago

The "default" I2CDevice uses synchronized blocks around operations, but that implementation does not actually provide any sort of thread-safety between devices - e.g. it is possible to run a multi-threaded program that uses two or more different I2C devices and have the I2C operations conflict with each other (the current use of synchronized only works for an instance of a class, and using the internal delegate is likewise not helpful since that is also instantiated at the provider level).

Thread safety on other levels of operations may also need to be examined, but at this point, no other points have been identified.

EAGrahamJr commented 1 year ago

(Deleted previous comments - those errors were a result of overloading the impedance on the I2C bus, which caused odd comm errors.)

EAGrahamJr commented 1 year ago

Hm. Note that I haven't been able to actually generate a conflict, so it appears this can be either closed or low-priority.

mattjlewis commented 1 year ago

I initially decided that this wasn't a problem that diozero should attempt to solve and be entirely left to the developer / device owner. As you say, there's synchornization of communications on the bus itself, and not just the threads within a diozero application but across other applications. On reflections, maybe diozero should at least synchronize communications on the bus, across devices - internally manage and synchronize using some sort of communication channel instance. This could then be reused across I2C, SPI and Serial comms.

EAGrahamJr commented 1 year ago

Just thought of this: it might make the library less portable (e.g. only suitable for platforms that support threads).

Stuff to ponder...

mattjlewis commented 1 year ago

I'll add the generic internal communication channel mutex idea to the backlog. Would be useful for a multi-threaded Java app at the very least.

EAGrahamJr commented 1 year ago

Updated information: I've been running a multi-threaded multi-I2C app for several months without any issues on a Raspberry Pi. I sincerely doubt that I've managed to completely avoid all collisions in that time, so it's possible the native I/O is providing blocking in that case?

EAGrahamJr commented 12 months ago

From a StackOverflow answer:

I2C is a shared bus with multiple devices, which could be accessed from multiple processes as well as threads. So the Linux I2C driver code uses a mutex to manage access to each I2C bus.

Both SMBus and the direct I2C access code in the kernel acquire locks.

Closing