Upgrade JNA to 5.10.0 #80617

DaveCTurner · 2021-11-10T15:58:05Z

This commit upgrades JNA to 5.10.0. The primary reason for this upgrade
is to adopt a newer version of libffi which supports the
LIBFFI_TMPDIR environment variable so that we can change the location
of temporary executables and resolve #77014.

This commit also switches to the upstream JNA releases rather than using
the specially-repackaged ones we've used in the past.

This commit upgrades JNA to 5.10.0. The primary reason for this upgrade is to adopt a newer version of `libffi` which supports the `LIBFFI_TMPDIR` environment variable so that we can change the location of temporary executables and resolve elastic#77014. This commit also switches to the upstream JNA releases rather than using the specially-repackaged ones we've used in the past.

DaveCTurner · 2021-11-10T16:02:40Z

The two main discussion points are:

Are we ok with dropping our special snowflake repackaging? This was more useful in the past, but these days it only exists to remove some binaries for unsupported platforms, and it seems like an awful lot of ceremony for very little benefit.
Are we ok with backporting this to 7.16 and 8.0? It should be a fairly low-risk upgrade but no upgrades carry zero risk. If we don't, we'll have to live with LIBFFI_TMPDIR should point somewhere that permits executables #77014 for a while or find an alternative workaround.

DaveCTurner · 2021-11-10T16:39:56Z

@elasticmachine please run elasticsearch-ci/part-1

elasticmachine · 2021-11-10T17:13:32Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

jasontedor · 2021-11-10T18:41:21Z

I'll copy my thoughts from our internal Slack discussion about this here. I'm supportive of this change. Historically, we created the custom JNA build for two reasons:

There was a time when the upstream JNA build process included a dependency on a newer version of glibc than was supported on some of the OS that we support (e.g., CentOS 6). Therefore, we created the custom JNA build process to recompile the native libraries for x86_64 Linux against an older version of glibc so that we could continue to support those older OS.
As @DaveCTurner mentioned, we also used this custom JNA build to remove from the packaging the native libraries for OS that we do not support.

The recompilation of the x86_64 Linux libraries is no longer needed, as JNA upstream reverted their build process so that they could also continue to support the older OS. Therefore, we don't need 1. anymore, and removed that from our custom JNA build.

We kept 2. around, but I don't think we should anymore. I think that if we want to take a stance against explicitly not supporting OS that we do not support, it should not be through how we package a dependency, but rather done more explicitly (please note the "if" in that sentence, I'm not taking a stance one way or the other on whether or not we should, only that if we decide that's a stance to take, it shouldn't be done in this way, but more explicitly).

Taken together, I do think we should drop our custom JNA build, and use JNA upstream directly. There's a lot of effort that goes into maintaining our custom JNA build, publishing etc. every time we want to bump the version on our upstream dependency, etc., and it's not worth it solely to implicitly indicate lack of support for certain OS/architectures combinations.

mark-vieira · 2021-11-10T18:51:51Z

build-tools-internal/build.gradle

@@ -211,7 +211,7 @@ dependencies {
  api 'com.netflix.nebula:gradle-extra-configurations-plugin:5.0.1'
  api 'com.netflix.nebula:gradle-info-plugin:9.2.0'
  api 'org.apache.rat:apache-rat:0.11'
-  api "org.elasticsearch:jna:5.7.0-1"
+  api 'net.java.dev.jna:jna:5.10.0'


We should actually just replace this with versions.getProperty('jna') to avoid having this duplicated in two places.

Thanks, see e58285b

mark-vieira · 2021-11-10T18:52:12Z

build-tools-internal/version.properties

@@ -18,8 +18,8 @@ log4j             = 2.11.1
 slf4j             = 1.6.2
 ecsLogging        = 1.2.0

-# when updating the JNA version, also update the version in buildSrc/build.gradle
-jna               = 5.7.0-1
+# when updating the JNA version, also update the version in build-tools-internal/build.gradle


If we leverage versions in the reference build script instead we can ditch this comment since updating it here will update the build script as well.

👍 done in e58285b

mark-vieira · 2021-11-10T18:54:52Z

Taken together, I do think we should drop our custom JNA build, and use JNA upstream directly. There's a lot of effort that goes into maintaining our custom JNA build, publishing etc. every time we want to bump the version on our upstream dependency, etc., and it's not worth it solely to implicitly indicate lack of support for certain OS/architectures combinations.

👍. Maintaining forks is a big pain, to the degree we can use the original upstream dependencies we should. I don't see compelling motivation to continue to maintain our custom build.

We kept 2. around, but I don't think we should anymore. I think that if we want to take a stance against explicitly not supporting OS that we do not support, it should not be through how we package a dependency, but rather done more explicitly (please note the "if" in that sentence, I'm not taking a stance one way or the other on whether or not we should, only that if we decide that's a stance to take, it shouldn't be done in this way, but more explicitly).

I'm not sure there's any immediate action on this right now. If the upstream dependency "supports" OSes we don't I don't see that as a problem. We don't really strictly enforce any OS requirements right now, so as with other things if it "works" but is unsupported then that's basically the status quo for many other scenarios (i.e. BSD).

mark-vieira

LGTM

pugnascotia · 2021-11-10T20:12:28Z

server/build.gradle

@@ -62,7 +62,7 @@ dependencies {
  api "org.apache.logging.log4j:log4j-core:${versions.log4j}", optional

  // repackaged jna with native bits linked against all elastic supported platforms


Is this comment now redundant?

Ah so it is, removed in 5a0d091

DaveCTurner · 2021-11-10T22:25:32Z

@elasticmachine please run elasticsearch-ci/part-2

This commit upgrades JNA to 5.10.0. The primary reason for this upgrade is to adopt a newer version of `libffi` which supports the `LIBFFI_TMPDIR` environment variable so that we can change the location of temporary executables and resolve elastic#77014. This commit also switches to the upstream JNA releases rather than using the specially-repackaged ones we've used in the past.

Today if `libffi` cannot allocate pages of memory which are both writeable and executable then it will attempt to write code to a temporary file. Elasticsearch configures itself a suitable temporary directory for use by JNA but by default `libffi` won't find this directory and will try various other places. In certain configurations, none of the other places that `libffi` tries are suitable. With older versions of JNA this would result in a `SIGSEGV`; since elastic#80617 the JVM will exit with an exception. With this commit we use the `LIBFFI_TMPDIR` environment variable to configure `libffi` to use the same directory as JNA for its temporary files if they are needed. Closes elastic#18272 Closes elastic#73309 Closes elastic#74545 Closes elastic#77014 Closes elastic#77053 Relates elastic#77285 Co-authored-by: Rory Hunter <roryhunter2@gmail.com>

Today if `libffi` cannot allocate pages of memory which are both writeable and executable then it will attempt to write code to a temporary file. Elasticsearch configures itself a suitable temporary directory for use by JNA but by default `libffi` won't find this directory and will try various other places. In certain configurations, none of the other places that `libffi` tries are suitable. With older versions of JNA this would result in a `SIGSEGV`; since #80617 the JVM will exit with an exception. With this commit we use the `LIBFFI_TMPDIR` environment variable to configure `libffi` to use the same directory as JNA for its temporary files if they are needed. Closes #18272 Closes #73309 Closes #74545 Closes #77014 Closes #77053 Relates #77285 Co-authored-by: Rory Hunter <roryhunter2@gmail.com>

Today if `libffi` cannot allocate pages of memory which are both writeable and executable then it will attempt to write code to a temporary file. Elasticsearch configures itself a suitable temporary directory for use by JNA but by default `libffi` won't find this directory and will try various other places. In certain configurations, none of the other places that `libffi` tries are suitable. With older versions of JNA this would result in a `SIGSEGV`; since elastic#80617 the JVM will exit with an exception. With this commit we use the `LIBFFI_TMPDIR` environment variable to configure `libffi` to use the same directory as JNA for its temporary files if they are needed. Closes elastic#18272 Closes elastic#73309 Closes elastic#74545 Closes elastic#77014 Closes elastic#77053 Relates elastic#77285 Co-authored-by: Rory Hunter <roryhunter2@gmail.com>

Today if `libffi` cannot allocate pages of memory which are both writeable and executable then it will attempt to write code to a temporary file. Elasticsearch configures itself a suitable temporary directory for use by JNA but by default `libffi` won't find this directory and will try various other places. In certain configurations, none of the other places that `libffi` tries are suitable. With older versions of JNA this would result in a `SIGSEGV`; since #80617 the JVM will exit with an exception. With this commit we use the `LIBFFI_TMPDIR` environment variable to configure `libffi` to use the same directory as JNA for its temporary files if they are needed. Closes #18272 Closes #73309 Closes #74545 Closes #77014 Closes #77053 Relates #77285 Co-authored-by: Rory Hunter <roryhunter2@gmail.com> Co-authored-by: Rory Hunter <roryhunter2@gmail.com>

* Set LIBFFI_TMPDIR at startup (#80651) Today if `libffi` cannot allocate pages of memory which are both writeable and executable then it will attempt to write code to a temporary file. Elasticsearch configures itself a suitable temporary directory for use by JNA but by default `libffi` won't find this directory and will try various other places. In certain configurations, none of the other places that `libffi` tries are suitable. With older versions of JNA this would result in a `SIGSEGV`; since #80617 the JVM will exit with an exception. With this commit we use the `LIBFFI_TMPDIR` environment variable to configure `libffi` to use the same directory as JNA for its temporary files if they are needed. Closes #18272 Closes #73309 Closes #74545 Closes #77014 Closes #77053 Relates #77285 Co-authored-by: Rory Hunter <roryhunter2@gmail.com> * Fix incorrect SSL usage Co-authored-by: Rory Hunter <roryhunter2@gmail.com>

In elastic#80617 we upgraded to a version of JNA which would report the message `Failed to allocate closure` if the temp dir for `libffi` did not permit executables (rather than crashing with a segfault). This commit extends the docs to mention this message.

In #80617 we upgraded to a version of JNA which would report the message `Failed to allocate closure` if the temp dir for `libffi` did not permit executables (rather than crashing with a segfault). This commit extends the docs to mention this message.

In elastic#80617 we upgraded to a version of JNA which would report the message `Failed to allocate closure` if the temp dir for `libffi` did not permit executables (rather than crashing with a segfault). This commit extends the docs to mention this message.

In #80617 we upgraded to a version of JNA which would report the message `Failed to allocate closure` if the temp dir for `libffi` did not permit executables (rather than crashing with a segfault). This commit extends the docs to mention this message.

由于在 7.16.0 之后使用 JNA 5.10.0 elastic/elasticsearch#80617, 默认编码 `native.encoding` 会是 ASCII-US 这会导致转码问题 (https://github.com/java-native-access/jna/blob/5.10.0/src/com/sun/jna/Native.java#L129C45-L129C45) 故设定对应的编码为 UTF-8 (https://github.com/java-native-access/jna/blob/5.10.0/src/com/sun/jna/Native.java#L846C3-L846C3) 以解决此问题.

DaveCTurner added :Core/Infra/Core Core issues without another label >upgrade v8.0.0 v7.16.1 v8.1.0 labels Nov 10, 2021

DaveCTurner added team-discuss Team:Core/Infra Meta label for core/infra team labels Nov 10, 2021

DaveCTurner marked this pull request as ready for review November 10, 2021 17:13

mark-vieira requested changes Nov 10, 2021

View reviewed changes

DaveCTurner added 2 commits November 10, 2021 19:03

Merge branch 'master' into 2021-11-10-jna-5.10

f470511

Eliminate duplication

e58285b

DaveCTurner requested a review from mark-vieira November 10, 2021 19:09

mark-vieira approved these changes Nov 10, 2021

View reviewed changes

pugnascotia reviewed Nov 10, 2021

View reviewed changes

pugnascotia approved these changes Nov 10, 2021

View reviewed changes

Redundant comment

5a0d091

DaveCTurner added auto-backport-and-merge and removed team-discuss labels Nov 11, 2021

DaveCTurner merged commit e8d8b12 into elastic:master Nov 11, 2021

DaveCTurner deleted the 2021-11-10-jna-5.10 branch November 11, 2021 07:20

DaveCTurner mentioned this pull request Nov 11, 2021

[8.0] Upgrade JNA to 5.10.0 (#80617) #80646

Merged

DaveCTurner mentioned this pull request Nov 11, 2021

Set LIBFFI_TMPDIR at startup #80651

Merged

DaveCTurner mentioned this pull request Nov 15, 2021

Set LIBFFI_TMPDIR at startup (#80651) #80698

Merged

DaveCTurner mentioned this pull request Nov 15, 2021

Set LIBFFI_TMPDIR at startup (#80651) #80699

Merged

danhermann added v7.16.0 and removed v7.16.1 labels Nov 16, 2021

mark-vieira added v8.0.0-rc1 and removed v8.0.0 labels Jan 12, 2022

DaveCTurner mentioned this pull request Feb 8, 2022

Note libffi error message in tmpdir docs #83662

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade JNA to 5.10.0 #80617

Upgrade JNA to 5.10.0 #80617

DaveCTurner commented Nov 10, 2021

DaveCTurner commented Nov 10, 2021 •

edited

Loading

DaveCTurner commented Nov 10, 2021

elasticmachine commented Nov 10, 2021

jasontedor commented Nov 10, 2021

mark-vieira Nov 10, 2021

DaveCTurner Nov 10, 2021 •

edited

Loading

mark-vieira Nov 10, 2021

DaveCTurner Nov 10, 2021

mark-vieira commented Nov 10, 2021 •

edited

Loading

mark-vieira left a comment

pugnascotia Nov 10, 2021

DaveCTurner Nov 10, 2021

DaveCTurner commented Nov 10, 2021

		@@ -62,7 +62,7 @@ dependencies {
		api "org.apache.logging.log4j:log4j-core:${versions.log4j}", optional

		// repackaged jna with native bits linked against all elastic supported platforms

Upgrade JNA to 5.10.0 #80617

Upgrade JNA to 5.10.0 #80617

Conversation

DaveCTurner commented Nov 10, 2021

DaveCTurner commented Nov 10, 2021 • edited Loading

DaveCTurner commented Nov 10, 2021

elasticmachine commented Nov 10, 2021

jasontedor commented Nov 10, 2021

mark-vieira Nov 10, 2021

Choose a reason for hiding this comment

DaveCTurner Nov 10, 2021 • edited Loading

Choose a reason for hiding this comment

mark-vieira Nov 10, 2021

Choose a reason for hiding this comment

DaveCTurner Nov 10, 2021

Choose a reason for hiding this comment

mark-vieira commented Nov 10, 2021 • edited Loading

mark-vieira left a comment

Choose a reason for hiding this comment

pugnascotia Nov 10, 2021

Choose a reason for hiding this comment

DaveCTurner Nov 10, 2021

Choose a reason for hiding this comment

DaveCTurner commented Nov 10, 2021

DaveCTurner commented Nov 10, 2021 •

edited

Loading

DaveCTurner Nov 10, 2021 •

edited

Loading

mark-vieira commented Nov 10, 2021 •

edited

Loading