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

Add JNI implementations for graalvm native-image compatibility #8

Merged
merged 7 commits into from
Jun 20, 2020
Merged

Add JNI implementations for graalvm native-image compatibility #8

merged 7 commits into from
Jun 20, 2020

Conversation

eatkins
Copy link
Contributor

@eatkins eatkins commented Jun 19, 2020

This PR reworks the project so that the client and server socket classes for posix and windows platforms have both jni and jna implementations. The reason for this is to support building a native sbt client using graalvm. JNA does not work well with the graalvm native-image tool.

eatkins added 6 commits June 19, 2020 14:07
The jna and graalvm native-image tool do not play nicely together.
Reflection is used extensively by the jna in a way that is difficult,
where possible, to get to work with native-image (posix I was able to
get working with some, but not all versions of the native-image tool,
windows I didn't even try because of the way it used proxy classes). In
order to support using ipcsocket in a native client for sbt, it is
necessary to provide jni implementations of the client and server
sockets for both posix and windows platforms.

In the case of the unix domain sockets, it was not that difficult to
extract an interface out of UnixDomainSocketLibrary that could be
provided both by the jna and jni because there few jna provided structs
that needed to be passed around. This was not the case with windows,
where there were more instances of things like SecurityAttributes and
OVERLAPPED structs floating around as well as numerous constants that
were used in the regular java code. The necessary refactoring was more
extensive and it was significantly more difficult to extract the shared
interface. Nevertheless, this was done for both.

The native libraries that provide the jni implementations are provided
in a platform specific location in the library's resources. Building is
done entirely within sbt and does not rely on make or another third
party build tool. The library is quite small for windows and linux < 20K
each, but, unfortunately, is somewhat bloated at ~350K for windows. I
think it should be possible to get the sized down to around 20K for
windows as well but I had problems building a freestanding executable
with mingw*. The native libraries are also built on appveyor and
uploaded as artifacts, making it possible to update all of the native
libraries without relying on building on a developer's machine or vm.
Getting a compilation toolchain working on windows can be a pain, so the
library can be cross-compiled for windows using mingw on either osx or
linux (the cross-compilation is done using linux in ci).

Each of the server and client class constructors now has an addition
useJNI parameter that can be used to select the implementation at
runtime. When JNI is used, the library will extract the appropriate
shared library into a temporary directory that is configurable via
system property. After loading the library, it spawns a background
thread to delete any old temporary shared librarys in the temporary
directory to try and avoid filling the user's hard drive. I didn't
bother with a shutdown hook because it doesn't really work with windows
since you can't delete a file that is being used which would require
that the shutdown hook close the classloader for the native library
which could cause another shutdown hook to fail. This approach avoids
that issue but does introduce an issue with posix systems where it would
be possible for concurrent processes to delete their respective copies
of the shared library. I avoid this by writing a pid file to a
corresponding file to the temporary shared library itself. We only try
to delete the shared libraries in the directory if the corresponding pid
is not alive. On windows, there is no need for a pid file because the os
won't let us delete the file if it is open anyway.

I consolidated the unix and windows tests into a single SocketTest that
selects the correct implementation based on the runtime platform.

There are unfortunately some formatting changes that got incorporated
into this commit when I ran javafmt but it isn't worth the effort to
extract them into a separate commit.

* Much of the native infrastructure was adapted from
https://github.com/swoval/swoval. For that library, I was able to build
a freestanding library with mingw that was roughly 10K but, for reasons I
don't understand, it didn't work with this project and I opted not to
spend a ton of time reducing the size of the binary.
In 986a33c, we started adding access control to the named pipes created
for windows server sockets. This was designed to protect snooping across
different user accounts. The security choice made was to restrict client
connections to the login session. The drawback to this choice was that
it prevents remote ssh connections to the same account from accessing
the named pipe. This commit makes the security level configurable and
keeps the default security level to be the logon dacl. sbt, which is the
primary downstream consumer of this library, will likely be updated to
use the owner dacl by default while allowing users to also choose to use
the more restrictive logon dacl option or forgo security entirely if they wish.
This provides formatters for the project *.sbt files, *.java source
files and *.c jni source files.
@eed3si9n
Copy link
Member

The title of this PR is "GraalVM support", but basically this PR is more like "Add JNA implementation"?

Also how does this affect the build / release process? Is there going to be a single universal JAR that can handle all platforms, or do we need to build one per platform?

@eatkins eatkins changed the title GraalVM support Add JNI implementation for graalvm native-image compatibility Jun 20, 2020
@eatkins eatkins changed the title Add JNI implementation for graalvm native-image compatibility Add JNI implementations for graalvm native-image compatibility Jun 20, 2020
@eatkins
Copy link
Contributor Author

eatkins commented Jun 20, 2020

The title of this PR is "GraalVM support", but basically this PR is more like "Add JNA implementation"?

Title changed.

Also how does this affect the build / release process? Is there going to be a single universal JAR that can handle all platforms, or do we need to build one per platform?

This shouldn't require major changes to either building or releasing the library. New versions of the native libraries are built and uploaded to appveyor with each PR:
https://ci.appveyor.com/project/sbt/ipcsocket/build/job/k0lpl67j8avs6gu9/artifacts
https://ci.appveyor.com/project/sbt/ipcsocket/build/job/lcit981famoq1acy/artifacts
These libraries are checking into src/main/resources so they are automatically packaged as part of jar building. The single jar contains all three implementation libraries so it is still a universal library though a bit bloated by the presence of platform specific files.

The safest way to make a release would be to download the binaries generated by the last commit and then check them in to a new commit from which the release is actually made. This step isn't strictly necessary though if no changes have been made to the *.c files since the last release.
The artifacts contain some content that changes every time they are rebuilt so if we update the binaries when there have been no *.c source file changes, we will see a binary diff. This isn't harmful. Both binaries will work equivalently but it does increase the size of the git repository. Given how rarely we release ipcsocket, this is unlikely to ever matter so the most prudent thing is to just check in the latest binaries whenever we make a release.

@eed3si9n
Copy link
Member

The safest way to make a release would be to download the binaries generated by the last commit and then check them in to a new commit from which the release is actually made. This step isn't strictly necessary though if no changes have been made to the *.c files since the last release.

ok. If that's a manual process, could we add an instruction in DEVELOPING.md?

@eatkins
Copy link
Contributor Author

eatkins commented Jun 20, 2020

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

Successfully merging this pull request may close these issues.

2 participants