Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Q: When OpenJDK related changes expected to be released? #156

Closed
yuriibratchuk opened this issue Apr 6, 2020 · 22 comments
Closed

Q: When OpenJDK related changes expected to be released? #156

yuriibratchuk opened this issue Apr 6, 2020 · 22 comments

Comments

@yuriibratchuk
Copy link

Hi Kevin @madhephaestus ,
I see that you are the most recent maintainer and very active contributor, so let me first say Thank you for your great work & contribution to this awesome library! ;)

There is a fix for OpenJDK support has been already submitted and available in java11 branch. It might be very important for the users who are switching away from Oracle's Java.

The last release of nrjavaserial, 3.15.0 was in November of 2018, more than a year ago.

When is it planned to release these OpenJDK-related changes?

Thank you!

Best regards,
Yurii

Referenced:
openjdk11 support
Release schedule?

@madhephaestus
Copy link
Member

Given the state of the world, i have a little time to take a look at this finally. Sorry for the delay all!

madhephaestus added a commit that referenced this issue Apr 7, 2020
@madhephaestus
Copy link
Member

@yuriibratchuk
Copy link
Author

@madhephaestus , thanks a lot for delivering so quick release! 👍
I've checked the released changes for 4.0.0 and 4.0.1 and see that changes from #133 were not included...

Could you please check?

Thank you!

@madhephaestus
Copy link
Member

@yuriibratchuk what OS?

@madhephaestus madhephaestus reopened this Apr 8, 2020
@yuriibratchuk
Copy link
Author

@madhephaestus Windows 10

madhephaestus added a commit that referenced this issue Apr 8, 2020
@madhephaestus
Copy link
Member

@yuriibratchuk published 4.0.2 and 3.16.1 with the windows update

@yuriibratchuk
Copy link
Author

yuriibratchuk commented Apr 9, 2020

@madhephaestus , thanks a lot for the quick update and new releases! ;)

Regard Java 8 - specific release, I've notice that in the latest (on this moment) release 3.18.0 https://github.com/NeuronRobotics/nrjavaserial/blob/3.18.0/src/main/c/src/SerialImp.c still contains get_java_var_long an "old" return type. According to #133 the type has changed from long to size_t.

Could you pls clarify, is it expected to keep OpenJDK-related in 3.x releases?

Just curious about these important changes for the users that might not be able to switch their projects quickly to java 11.

Thank you in advance!

@madhephaestus
Copy link
Member

@yuriibratchuk I made a merged release, its compiled in java 8 and incorporates the java11 commits. Can you check to see if 3.19.0 works in your java11 build?

@wborn
Copy link
Contributor

wborn commented Apr 9, 2020

Is the only difference between 3.19.0 and 4.2.0 now that 4.2.0 is compiled with Java 11 whereas 3.19.0 with Java 8 @madhephaestus? So 3.19.0 has the Java 11 compatibility fix so it works on Java 8/11? I'm trying to figure out what version we want to use with openHAB 3 which will require Java 11. 😉

@madhephaestus
Copy link
Member

yes, thats the only difference now. I am not sure that it is important, so i want someone running a java11 deployment to try 3.19.0. If we get the OK then the 4.x branch of java 11 only can die. Ideally we have just 3.19.0 as the single source moving forward.

@wborn
Copy link
Contributor

wborn commented Apr 9, 2020

OK thanks for the info! I will try 3.19.0 on Java 11 then and see how well it works. 👍 Usually I test linux64 and armhf (RPi). But with a bit of effort I can also test Windows in a VM and run a arm64 image on the same RPi.

I also saw you made some changes for the lockfile in 53a85c6. Does it still use the same kind of lockfiles now because -DLIBLOCKDEV / -llockdev was removed? Was that change just because the liblockdev1-dev package was renamed to liblockfile-dev? It may relate to #60.

@madhephaestus
Copy link
Member

i have liblockfile installed and can not get the old DLIBLOCKDEV code to compile. It was only being used on the linux systems and was causing compatibility issues as it was changed at the OS level. I would be happy to revisit this issue in #60 but would like the fix to be cross platform and not depend on a system dep.

@wborn
Copy link
Contributor

wborn commented Apr 9, 2020

I don't mind that the lockfiles are no longer enabled because we also had users having issues with it. To mitigate that we already used a recompiled version in which lockfile support was disabled.

@wborn
Copy link
Contributor

wborn commented Apr 12, 2020

While testing 3.19.0 on Linux x86_64 I noticed there are still lock files being generated/used in /run/lock. So maybe not all (Linux) .so files were updated for this change @madhephaestus?

It should be using the regular Linux x86_64 .so file:

$ lsof userdata/tmp/libNRJavaSerial_wouter_0/*
COMMAND   PID   USER  FD   TYPE DEVICE SIZE/OFF     NODE NAME
java    15041 wouter mem    REG  259,2    75640 46007374 userdata/tmp/libNRJavaSerial_wouter_0/libNRJavaSerial.so
$ md5sum userdata/tmp/libNRJavaSerial_wouter_0/*
a80c5d3149c704f9253c07199e38706a  userdata/tmp/libNRJavaSerial_wouter_0/libNRJavaSerial.so

@madhephaestus
Copy link
Member

I just recompiled master and checked the binaries, no change.

I also removed liblockfile-dev and recompiled, so there is no possible way the dep is being used. The compile worked, and the binaries are identical. AFAIK, RXTX had always used lock files, but had not used a lockfile library. We added the library, but this caused compile issues across the platforms. The failover when the lockfile library is not availible, it uses the old built in lock file system. Is this the behavior your seeing?

@wborn
Copy link
Contributor

wborn commented Apr 12, 2020

That indeed explains it. I wasn't aware there was still a build in lock file system being used.
We currently use a recompiled version using -DDISABLE_LOCKFILES instead of -DLIBLOCKDEV and without -llockdev. But your changes obviously didn't add -DDISABLE_LOCKFILES.

@madhephaestus
Copy link
Member

madhephaestus commented Apr 12, 2020

are the lockfiles causing an issue? should they be disabled on all platforms?

@wborn
Copy link
Contributor

wborn commented Apr 12, 2020

I will do some more testing to see if the build in lockfiles are really causing us issues. It's better to have some sort of locking mechanism in place instead of none to prevent issues.

@wborn
Copy link
Contributor

wborn commented Apr 13, 2020

I've completed testing 3.19.0 with Java 11 on Linux (x86_64, armhf, arm64) and Windows (64-bit). It seems to work fine on those platforms and doesn't crash the Java 11 VM. 👍 So I think there's currently not really a need for maintaining the 4.x version.

The only real issue I ran into (Linux x86_64) was that a few times the following error was printed:

initialise_event_info_struct: initialise failed! (similar to issue #111)

See code:

report_error("initialise_event_info_struct: initialise failed!\n");

It didn't crash the JVM, but afterwards the serial connection couldn't be used. I don't know exactly what the root cause is or how to reproduce it. It occurred once after removing a connection and then recreating it. But I couldn't reproduce it this way afterwards.

The built in lockfiles seem to work pretty well. They didn't cause issues with Docker or after killing the application. It would just print "Removing stale lock file" and continue after restarting the application.

Sometimes I do see some warnings regarding ports already been locked:

RXTX fhs_lock() Error: opening lock file: /var/lock/LCK..ttyUSB0: File exists. It is mine

I know how to reproduce that so I will look into if it can be prevented by rewriting some code in openHAB because it can be disturbing for end users when it occurs frequently.

@wborn
Copy link
Contributor

wborn commented Apr 13, 2020

I had a look at our code and it doesn't seem we can prevent the warning ("RXTX fhs_lock() Error: opening lock file: /var/lock/LCK..ttyUSB0: File exists. It is mine"). It is always printed whenever there is a port open (locked) and we call CommPortIdentifier.getPortIdentifiers() while gnu.io.rxtx.SerialPorts isn't set. In that case we want it to register scanned ports so we know what all available ports are:

/*
First try to register ports specified in the properties
file. If that doesn't exist, then scan for ports.
*/
if (!registerSpecifiedPorts(CommPortIdentifier.PORT_SERIAL)) {
if (!registerKnownPorts(CommPortIdentifier.PORT_SERIAL)) {
registerScannedPorts(CommPortIdentifier.PORT_SERIAL);
}
}

@madhephaestus
Copy link
Member

Great!! @wborn Thank you for the testing, im very glad to hear we can move forward with the single set of sources.

As for the print issue, maybe that should be its own issue?

@wborn
Copy link
Contributor

wborn commented Apr 13, 2020

It's probably best to do that since it has been there in previous versions as well. I've created #166 for this @madhephaestus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants