Repository navigation
Conversation
- Replace 'select' by 'poll' to fix segfaults when fd >= 1024 - Log a few error cases which we concealed in old code. - Fix some concurrency issues. (wip 4a6ff6c)
|
Thanks. I'd also ask you to read the code and suggestions here: #78. Note, we're short on volunteers here to help merge, so I do apologize for any delay before this hits our master branch. |
|
Thx for the hint 👍 Seems as we need a few more tweaks then. Will give it a try. |
b23270e to
ddc359c
Compare
- Reword some comments. - Drop logging. - Use 'select' as fallbck for OsX. But still allow to override using -DHAVE_POLL=1 gcc switch. - Revert accidentally checked-in libjssc.so. (wip 7e4e60e)
ddc359c to
ec9d759
Compare
There was a problem hiding this comment.
Is this part still needed? It seems like in this implementation we wait for data before returning in native code (either by poll or by select, even on Mac OS), so this sleep can be removed, if I understand everything correctly
jssc/src/main/java/jssc/SerialPort.java
Lines 1304 to 1310 in 597d06a
Fine by me. @GMKennedy and I will be making a new release this week, so I'd like to get this merged in asap if possible. For the record @GMKennedy did compile and test the code on MacOS and Linux with no observable regressions. |
|
@hiddenalpha, Just checking in on the |
GMKennedy
left a comment
There was a problem hiding this comment.
Code changes implement 'synchronized' function (ie monitor lock) which replaces thread sleep function for handling of contention/concurrency. SerialPort.java has been updated accordingly.
|
AFAIK, @GMKennedy's speaking specifically about this branch: https://github.com/java-native/jssc/commits/Poll-Update We'll add to this PR in the next day and merge. |
Sorry for not responding. But had no access to my account because I was not at work a few weeks. AFAIK this decision is superfluous meanwhile :) But honestly I even could not say if it should be removed or not. |
|
During our thorough testing (mac/linux/raspberry) of our improvements suggested in #126, we have never run into the issue of file descriptors with a value above 1024. However, it seems to be an issue on (some?) linux systems. Maybe you could have a look and improve our suggested code with your polling mechanism. I'm not sure if it is as portable as |
Yeah :( unfortunately poll is not as portable as select. This was the reason we kept select as a fallback.
Unluckily I do not know the exact reason why we ran into such FDs. I only saw that we had them. There were some suspicions that it could come for example from docker or that we use the lib in a jetty where half a dozen apps are running performing lots of file/network IO. |
One test per mistake that reached users before, so each fails if it comes back (JOURNAL.md now maps lesson -> test): - java-native#113 baud rate truncated by a cast: set every standard rate and read it back with stty (verified: reintroducing a (jbyte) cast fails it) - java-native#192 0-byte reads on a real open port - java-native#86/java-native#181 event carries its port; removeEventListener true, then false - java-native#107/java-native#181 closePort after the device vanished does not throw - java-native#90 a port opened above fd 1024 still reads (poll on Linux) - java-native#80 the library loads under a Turkish default locale - java-native#160 exceptions stay IOExceptions - java-native#51/java-native#65 tests load the native library just built, not another copy - java-native#110 class files have the configured target version - java-native#179/java-native#180/java-native#182 Linux port names incl. Moxa, and getPortNames(Pattern) VirtualPortRule gets unplug() (stop socat) and one executor per rule: the shared static executor was shut down after the first test, so a class could only hold one socat test. Two tests found open bugs, recorded in BACKLOG: - reading from an unplugged port hangs on Linux because read() returns 0 after a hangup and the native loop retries (B-04); that test is @ignore'd until the fix - an incremental build without clean leaves Java 9 class files, because the second compiler pass sees up-to-date classes (new B-25) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPFQMyWmP4EyGx33gCHP2r
One test per mistake that reached users before, so each fails if it comes back (JOURNAL.md now maps lesson -> test): - java-native#113 baud rate truncated by a cast: set every standard rate and read it back with stty (verified: reintroducing a (jbyte) cast fails it) - java-native#192 0-byte reads on a real open port - java-native#86/java-native#181 event carries its port; removeEventListener true, then false - java-native#107/java-native#181 closePort after the device vanished does not throw - java-native#90 a port opened above fd 1024 still reads (poll on Linux) - java-native#80 the library loads under a Turkish default locale - java-native#160 exceptions stay IOExceptions - java-native#51/java-native#65 tests load the native library just built, not another copy - java-native#110 class files have the configured target version - java-native#179/java-native#180/java-native#182 Linux port names incl. Moxa, and getPortNames(Pattern) VirtualPortRule gets unplug() (stop socat) and one executor per rule: the shared static executor was shut down after the first test, so a class could only hold one socat test. Two tests found open bugs, recorded in BACKLOG: - reading from an unplugged port hangs on Linux because read() returns 0 after a hangup and the native loop retries (B-04); that test is @ignore'd until the fix - an incremental build without clean leaves Java 9 class files, because the second compiler pass sees up-to-date classes (new B-25) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPFQMyWmP4EyGx33gCHP2r
HINT: I did not add recompiled libs as I only could provide/test the linux lib
anyway. Not sure how this gets handled in this git repo. Let me know.
Edit: (@tresf)