NeuronRobotics / nrjavaserial

A Java Serial Port system. This is a fork of the RXTX project that uses in jar loading of the native code.
Other
345 stars 143 forks source link

openjdk 11 support #131

Closed irfman12 closed 4 years ago

irfman12 commented 5 years ago

Using, trying to run against openjdk 11, getting, on 3.9.3 hs_err_pid16196.log

attached thread dump, any advice?

urrent thread (0x0000027c5ad57800): JavaThread "Thread-11" [_thread_in_native, id=11856, stack(0x000000faf1100000,0x000000faf1200000)]

Stack: [0x000000faf1100000,0x000000faf1200000], sp=0x000000faf11fe890, free space=1018k Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code) C [libNRJavaSerial.dll+0x753d]

Java frames: (J=compiled Java code, j=interpreted, Vv=VM code) j gnu.io.RXTXPort.readArray([BII)I+0 j gnu.io.RXTXPort$SerialInputStream.read([BII)I+278 j gnu.io.RXTXPort$SerialInputStream.read([B)I+66 j com.dls.jpos.transport.DLSSerialPort.serialEvent(Lgnu/io/SerialPortEvent;)V+251 j gnu.io.RXTXPort.sendEvent(IZ)Z+818 v ~StubRoutines::call_stub j gnu.io.RXTXPort.eventLoop()V+0 j gnu.io.RXTXPort$MonitorThread.run()V+21 v ~StubRoutines::call_stub

siginfo: EXCEPTION_ACCESS_VIOLATION (0xc0000005), reading address 0xfffffffff11ff388

ggmuelle commented 5 years ago

Hi, I've found the problem and have a fix (see attached patch): The function get_java_var_long() in SerialImp.c casts the Java-long-value to a long in case of type == 'J'. But Java-Long is 64 bit and C-long is only 32 bit. Later this long is casted to a 64-bit-memory-address -> crash. A possible solution is to return always size_t instead of long. I do not understand why this is not a problem with Java 8, but the fix works there too.

issue-131-patch.zip

irfman12 commented 5 years ago

Great, I will try this out and let you know the results, appreciate the investigation

irfman12 commented 5 years ago

I had to make changes to on the internal MakeFile, Java location, tcc-gdb 32 and 64. I think i got the it built. I think it's working now on JDK 11 change JDKDIR

make sure MINGW32 = C:\TDM-GCC-32 MINGW64 = C:\TDM-GCC-64

windows: mingw32-make -C .\src\main\c windowsLocal gradle build

madhephaestus commented 5 years ago

I made a java11 branch, we can put changes into there to stage the java 11 transition. Send java 11 specific changes there and if it doesn't break stuff we can make it mainline.

At this point most sources are java 1.6 compatible, and the build enforces 1.6 compilation.

ggmuelle commented 5 years ago

Thanks for your effort. Would please give me the permission to commit something into the Java 11 branch?

madhephaestus commented 5 years ago

the way GItHub works is you fork this repo using the button in the upper right corner. You will get your own copy of the repo to make changes to. When you are happy with your changes, submit a pull request like this: https://help.github.com/articles/creating-a-pull-request/ and it will be merged into the mainline after discussion and testing.

Make sure all your commits have this issue in them. You would have to have the string '#131' at the start of each commit (minus the quotation marks).

irfman12 commented 5 years ago

@ggmuelle I've added a pull request https://github.com/NeuronRobotics/nrjavaserial/pull/133

ggmuelle commented 5 years ago

@madhephaestus I've created a new pull request which includes the binaries.

scriptator commented 5 years ago

Probably the same problem when using Java 10:

Current thread (0x000001f84a048000):  JavaThread "Thread-8" [_thread_in_native, id=78560, stack(0x0000006fb8d00000,0x0000006fb8e00000)]

Stack: [0x0000006fb8d00000,0x0000006fb8e00000],  sp=0x0000006fb8dfeca0,  free space=1019k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
C  [libNRJavaSerial.dll+0x753d]

Java frames: (J=compiled Java code, j=interpreted, Vv=VM code)
j  gnu.io.RXTXPort.readArray([BII)I+0
j  gnu.io.RXTXPort$SerialInputStream.read([BII)I+187
j  gnu.io.RXTXPort$SerialInputStream.read([B)I+38
j  com.zactrack.loc8me.location_processor.driver.impl.SerialPort$SerialReader.run()V+50
j  java.lang.Thread.run()V+11 java.base@10.0.2
v  ~StubRoutines::call_stub

siginfo: EXCEPTION_ACCESS_VIOLATION (0xc0000005), reading address 0xffffffffb8cfea78
OS: Windows 10 , 64 bit Build 17763 (10.0.17763.1)
CPU:total 4 (initial active 4) (2 cores per cpu, 2 threads per core) family 6 model 142 stepping 9, cmov, cx8, fxsr, mmx, sse, sse2, sse3, ssse3, sse4.1, sse4.2, popcnt, avx, avx2, aes, clmul, erms, 3dnowpref, lzcnt, ht, tsc, tscinvbit, bmi1, bmi2, adx, fma
Memory: 4k page, physical 8281304k(2180328k free), swap 11558104k(4106048k free)
vm_info: Java HotSpot(TM) 64-Bit Server VM (10.0.2+13) for windows-amd64 JRE (10.0.2+13), built on Jun 28 2018 01:57:56 by "mach5one" with MS VC++ 12.0 (VS2013)

What's the easiest way of testing whether the fix works for my problem?

ggmuelle commented 5 years ago

Download the binaries from my fork and include it into to your nrjavaserial.jar.

wborn commented 5 years ago

Is this a Windows only issue?

It seems to work just fine for me:

JVM
  Java Virtual Machine        OpenJDK 64-Bit Server VM version 11.0.1+13
  Version                     11.0.1
  Vendor                      Oracle Corporation
Operating system
  Name                        Linux version 4.15.0-42-generic
  Architecture                amd64
  Processors                  8
irfman12 commented 5 years ago

My use case is windows cant speak for linux.

On Wed, Dec 12, 2018, 1:07 PM Wouter Born <notifications@github.com wrote:

Is this a Windows only issue?

It seems to work just fine for me:

JVM Java Virtual Machine OpenJDK 64-Bit Server VM version 11.0.1+13 Version 11.0.1 Vendor Oracle Corporation Operating system Name Linux version 4.15.0-42-generic Architecture amd64 Processors 8

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NeuronRobotics/nrjavaserial/issues/131#issuecomment-446685642, or mute the thread https://github.com/notifications/unsubscribe-auth/AAnYCmxlZmU1DU3bK8Bj35M8bq033A_xks5u4UXrgaJpZM4X661t .

Maia-Everett commented 5 years ago

This seems to be a duplicate of #116 and I reproduced the crash under 64-bit Windows with Java 9.

The fix in that issue fixed it for me.

I'm not actually sure that returning size_t (as done in the Java 11 branch) is correct, as that is an unsigned type. Returning jlong (as in the changes suggested in #116) would be safer, unless the developers are sure that the returned value will never need to be treated as a negative number.

loesler commented 5 years ago

I replaced the dll files inside the jar. No further crash of the jre occurs but I still get error, i.e.,

GetCommMOdemStatus failed!
GetCommMOdemStatus failed!
Error 0x1f at src/windows/termios.c(2611)
dbadia commented 5 years ago

Regarding #131 and #133 - is there a timeline to get an official release build with the fix? The bug affects Java 9 which was released a year and a half ago.

Jineric commented 4 years ago

Hello. Is there an update on this issue? We would like to be able to use this on our Java 11 upgrade.

minthaka commented 4 years ago

Hello! We are also interested in having a new, usable version for newer Javas. Would you be so kind to deliver us a new version of nrjavaserial? I've downloaded the Java 11 branch, and even build the project with Gradle, but it is not clear what to do with the generated RTXTcomm library. "Download the binaries from my fork and include it into to your nrjavaserial.jar."

cmoine commented 4 years ago

Download the binaries from my fork and include it into to your nrjavaserial.jar.

That is nice, however, I am using Gradle, I would like to properly retrieve the artifact from Maven Central, or whatever. Why this patch hasn't simply been integrated with a PR ? Would you like me to create a PR with @ggmuelle patch, or am I missing something ?

ocervinka commented 4 years ago

Hello, any change the java11 branch get merged to master or its artifacts gets to Maven Central? I moved from the legacy RXTX to NRJavaSerial some time ago because of Maven repository and because this library has binaries for many platforms bundled in jar, however now it's the only dependency which prevent our project being upgraded to Java 11.

madhephaestus commented 4 years ago

Please check https://github.com/NeuronRobotics/nrjavaserial/releases/tag/4.0.0

wborn commented 4 years ago

It's great to see a Java 11 compatible version for Windows! But would it also be possible to have a 4.x version with all commits that are on the master branch? It seems to be missing ARM64 support, named threads and the fix for the IllegalMonitorState.

madhephaestus commented 4 years ago

@wborn I added those updates and published 4.0.1

wborn commented 4 years ago

Many thanks @madhephaestus! :smile: :1st_place_medal:

madhephaestus commented 4 years ago

well... don't thank me till you check that it works @wborn ... I don't have any arm64 hardware to test on...