hedgecrw
commented
2 years ago Brief answers to your questions:
The global lock was added to the open and close methods because, under the hood in native code, these methods affect the global state/records of the serial ports along with the port enumeration methods getCommPorts() and getCommPort(String). Since the enumeration methods are synchronized class methods, it was necessary to also class-synchronize the open/close methods so that the library and ports did not get into weird states due to race conditions of users calling various combinations of these functions from different threads. (This was all based on bug reports, and the class synchronization fixed those bugs.) That being said, you are 100% correct that the critical section only involves the actual native function calls and not the entire open/close methods. As such, I've just checked in a fix which only class-synchronizes those specific native calls.
Regarding the join() method without a timeout, that's only partially true. The join without a timeout comes immediately after a timeout-enabled join fails, and the thread is then force-interrupted. Perhaps a better method would be to put the isAlive() and timed join() method calls in a loop so that it retries interrupting the thread if the first interrupt call fails, although I've never experienced this occurring in practice, nor are there any bug reports of that happening that I know of.
The bigger concern regarding what you posted on Issue #404, is that the native close method appears to be hanging and never returning. The only difference in the native close function between the current library version and 2.7.0 version (at least on Windows which it appears you are using) is a set of calls to cancel any awaiting I/O traffic on the serial ports before fully closing the port. I am guessing one of those might be hanging (although I'm not entirely sure why they would, but I have a suspicion on which call the culprit might be).
For starters, it appears you are testing using 2.8.0 which is several releases behind the current release of 2.9.0. Could you please retest using the latest version and verify that the problem still exists. If it does, would you be able to test a local beta JAR version if I send it to you with the suspecting offending call removed (as well as with the limited-scoped class synchronization)?
rusefillc
sprut666666
SerialPort has three kinds of blocking/threading
that's A LOT of complexity. Would it be possible to add code comments explaining the threading strategy of jSerialComm?
I am especially curious about https://github.com/rusefi/jSerialComm/commit/b52271c48cdc819b70ccb3960f97561a67954c7b which adds massive global lock without referencing specific use-cases of when that's beneficial or required.
if global lock is in fact needed I believe that the best practices are asking to reduce the protected section to the minimum. is the global lock protected section minimal at the moment? what are examples of entities/constraints which require global lock protection?