BACnet
BACnet copied to clipboard
ela-compil
BACnetClient is not thread safe
I have two threads in a same program, each thread is executed every seconds, they call ReadPropertyRequest function. First thread get a BinaryValue (0 or 1), second thread get an AnalogValue (between 20 and 30). Sometimes ReadPropertyRequest of BinaryValue return the AnalogValue! Can someone have a solution of my problem? PS: i call ReadPropertyRequest because COV does'nt seem to work for me
Hi Guys, I have same problem with https://github.com/ela-compil/BACnet/tree/v2-netstandard brunch. Could you please try to fix?
Unfortunatelly the client was not written to be thread safe. Try using a lock each time before you use it or write a wrapper that would do it for you for each atomic operation. Since BACnet operations have identifiers it should be possible to allow multiple requests to be sent out in the same time. I will leave this issue open to try to address it in the future.
Hi @gralin Thank you. Yes, it's working fine with locks. I have 1 thread per device and customer want to read properties and values from many devices. I'll try to create BACnet client per thread. Should work faster than locks.
Hey @minzdrav, just wondering if that worked for you? Did you manage to run a client per thread without issue?
Hey @minzdrav, just wondering if that worked for you? Did you manage to run a client per thread without issue?
Hi @MatthijsvdVeer Yes.
Yup, can confirm the reads are not threadsafe. It has to do with InvokeId, a uint8 that gets added to every request like a correlation ID, it gets incremented every request, and if you send too many requests it can quickly roll over and cause problems.
Update: On further investigation the reads cause a thread to be blocked waiting for reply because of the use of ManualResetEvent, this is actually a much bigger problem, as the non thread-safe client can be wrapped in a mutex, and multiple clients started in parallel, however, the reads will quickly eat up worker threads.
Fortunately a solution could be to replace ManualResetEvent with a TaskCompletionSource which doesn't block a thread. The rest of the code seems to be non-blocking which is great.
