JansiNativePty and JnaNativePty use illegal reflective access

[eatkins]

When I wire up sbt to use either a jna or jansi based system terminal on osx and start sbt with java 11, I get the following warning at startup:

WARNING: An illegal reflective access operation has occurred                                                                           
WARNING: Illegal reflective access by org.jline.terminal.impl.jansi.JansiNativePty (file:HOME/.sbt/boot/scala-2.12.12/org.scala-sbt/sbt/1.4.0-SNAPSHOT/jline-terminal-jansi-3.16.0.jar) to constructor java.io.FileDescriptor(int)
WARNING: Please consider reporting this to the maintainers of org.jline.terminal.impl.jansi.JansiNativePty                              
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations                                   
WARNING: All illegal access operations will be denied in a future release   

so I'm just doing what the warning told me to.

[gnodet]

We could use jdk.internal.misc.SharedSecrets.getJavaIOFileDescriptorAccess() if available and default to reflection if not.

[lvca]

Any news on this?

[gnodet]

Any news on this?

If you have any pointer / example on how to actually call this method, I'd be happy to give it another try. Or even better a PR where you can actually compile a method calling SharedSecrets. I can't find an easy way to access this package...

[jessebarnum]

I'm having the same problem:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jline.terminal.impl.exec.ExecTerminalProvider (file:/tmp/installer.jar) to constructor java.lang.ProcessBuilder$RedirectPipeImpl()
WARNING: Please consider reporting this to the maintainers of org.jline.terminal.impl.exec.ExecTerminalProvider
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

What do you mean by "If you have any pointer / example on how to actually call this method"? Which method?

[jessebarnum]

Unfortunately, I can no longer use jline because we're switching to Java 17, which doesn't allow you to override reflection errors like this. In my case, the problem stems from this code in ExecTerminalProvider:

    private ProcessBuilder.Redirect getRedirect(FileDescriptor fd) throws ReflectiveOperationException {
        // This is not really allowed, but this is the only way to redirect the output or error stream
        // to the input.  This is definitely not something you'd usually want to do, but in the case of
        // the `tty` utility, it provides a way to get
        Class<?> rpi = Class.forName("java.lang.ProcessBuilder$RedirectPipeImpl");
        Constructor<?> cns = rpi.getDeclaredConstructor();
        cns.setAccessible(true);
        ProcessBuilder.Redirect input = (ProcessBuilder.Redirect) cns.newInstance();
        Field f = rpi.getDeclaredField("fd");
        f.setAccessible(true);
        f.set(input, fd);
        return input;
    }

[gnodet]

I'm having the same problem:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jline.terminal.impl.exec.ExecTerminalProvider (file:/tmp/installer.jar) to constructor java.lang.ProcessBuilder$RedirectPipeImpl()
WARNING: Please consider reporting this to the maintainers of org.jline.terminal.impl.exec.ExecTerminalProvider
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

What do you mean by "If you have any pointer / example on how to actually call this method"? Which method?

jdk.internal.misc.SharedSecrets.getJavaIOFileDescriptorAccess()

[gnodet]

Unfortunately, I can no longer use jline because we're switching to Java 17, which doesn't allow you to override reflection errors like this. In my case, the problem stems from this code in ExecTerminalProvider:

    private ProcessBuilder.Redirect getRedirect(FileDescriptor fd) throws ReflectiveOperationException {
        // This is not really allowed, but this is the only way to redirect the output or error stream
        // to the input.  This is definitely not something you'd usually want to do, but in the case of
        // the `tty` utility, it provides a way to get
        Class<?> rpi = Class.forName("java.lang.ProcessBuilder$RedirectPipeImpl");
        Constructor<?> cns = rpi.getDeclaredConstructor();
        cns.setAccessible(true);
        ProcessBuilder.Redirect input = (ProcessBuilder.Redirect) cns.newInstance();
        Field f = rpi.getDeclaredField("fd");
        f.setAccessible(true);
        f.set(input, fd);
        return input;
    }

Have you configured the VM with the necessary --add-opens JVM command line arguments ?

--add-opens java.base/java.lang=ALL-UNNAMED

[jessebarnum]

That did fix the issue. Is there any better way to solve this?

[gnodet]

That did fix the issue. Is there any better way to solve this?

I'd be happy to be pointed to a better way.

Internally, the JDK use jdk.internal.misc.SharedSecrets.getJavaIOFileDescriptorAccess() that I pointed above, but I haven't really found a way to call this method without using reflection, as it's not in the advertised packages, and using reflection would lead to the exact same problems.

Maybe using some JNI. Acoording to the JNI Programmer's Guide and Specification says this in 10.9 Violating Access Control Rules:

The JNI does not enforce class, field, and method access control restrictions that can be expressed at the Java programming language level through the use of modifiers such as private and final. It is possible to write native code to access or modify fields of an object even though doing so at the Java programming language level would lead to an IllegalAccessException. JNI's permissiveness was a conscious design decision, given that native code can access and modify any memory location in the heap anyway.

I just found a library that may help with that, but it does not support a lof of platforms.

[eatkins]

Is the purpose of this to hide any text that may be printed to stdout or stderr? If so, you can use ProcessBuilder.Redirect.PIPE instead of ProcessBuilder.Redirect.INHERIT. I also wonder why you are shelling out at all to determine whether this is a system stream? Shelling out is a bit slow. Could each provider just implement its name in code?

[gnodet]

Is the purpose of this to hide any text that may be printed to stdout or stderr? If so, you can use ProcessBuilder.Redirect.PIPE instead of ProcessBuilder.Redirect.INHERIT.

This is a bit more complex. It basically executes tty <&1.

I also wonder why you are shelling out at all to determine whether this is a system stream? Shelling out is a bit slow. Could each provider just implement its name in code?

The point is to retrieve the real tty name, such as /dev/ttys004. This is not a static value.
The return value is not really used in the code, but it is used in the Diag helper

[eatkins]

Seems like it might not be worth the illegal reflexive access for a diagnostic string, but obviously that's not for me to decide. Maybe you could gate this on a system property so it would only affect users who are actually running diagnostics?

[gnodet]

Seems like it might not be worth the illegal reflexive access for a diagnostic string, but obviously that's not for me to decide. Maybe you could gate this on a system property so it would only affect users who are actually running diagnostics?

The value itself it not used at runtime (only in the Diag). However, the fact that the value is non null is important to determine if the stream to use for the terminal is a system stream or not. So it will be called anyway.

[eatkins]

Got it. This goes back to my original question though which is whether or not it is possible to identify whether it is a system stream without shelling out. Is a system stream a different class from other streams? Could it be?

[eatkins]

Also, it should be possible to shell out with fork/execve using the jna if you want to avoid jni. This should be relatively straightforward because execve has a simple api although I don't love the jna api itself (but jline already uses jna so at least it wouldn't be a new dependency). This would allow you to fully control the descriptors of the forked process. If you search for execve, you'll probably find c code that you can port to jna code. This would probably take the better part of a day to work through so I can understand not being motivated to do it. Alternatively, you could probably find the system function that tty calls under the hood and call that directly from the jna without needing to shell out.

[gnodet]

Also, it should be possible to shell out with fork/execve using the jna if you want to avoid jni. This should be relatively straightforward because execve has a simple api although I don't love the jna api itself (but jline already uses jna so at least it wouldn't be a new dependency). This would allow you to fully control the descriptors of the forked process. If you search for execve, you'll probably find c code that you can port to jna code. This would probably take the better part of a day to work through so I can understand not being motivated to do it. Alternatively, you could probably find the system function that tty calls under the hood and call that directly from the jna without needing to shell out.

There are 3 terminal providers: one using jansi, one using jna and one using fork/exec. I do recommend the jansi one, but if people want to avoid using JNI at all, the provider is here. Both jansi and jna use direct calls to the standard library to retrieve the ttyname. The exec provider has to rely on exec'ing tty <&1 to find out the same information without any JNI call. So there's no need to shell out if you use jansi or jna provider.

[gnodet]

@eatkins fwiw, your original issue is not about forking tty <&1, but the fact that both jansi and jna providers need to create FileDescriptor with the values obtained using the openpty native call. Those are needed in order to be able to create a FileInputStream on top of these native file descriptors.

[eatkins]

This comment #575 (comment) suggested that the issue is coming in the getRedirect method which is only used in systemStreamName in ExecTerminalProvider. But I see your point that setAccessible is used in the jansi and jna providers to make a FileDescriptor instance, which is ultimately used as the constructor argument for a File(Input|Output)Stream. There is a way around that by using custom InputStream and OutputStream classes that delegate to the read and write syscalls using JNA instead of File(Input|Output)Stream. I understand why that might be unappealing though.

That all being said, I do not see any warnings when I start sbt with java 19 which makes a system terminal without setting JNA or Jansi so I guess this is only a problem if you specify JNA, Jansi or Exec terminal.

I'm going to bow out of this discussion because I no longer use any software that relies on JLine, but good luck.

[gnodet]

#825 provides a fix for the original problem mentioned in this issue, the fact that JansiNativePty and JnaNativePty use illegal reflective access. It can be controlled using a few new system properties. The default method is to reflection if possible. If this fails, the native library is loaded as a workaround. The reflection requires the --add-opens java.base/java.io=ALL-UNNAMED JVM option to be set.

If needed, other methods could be added, such as using SharedSecrets.getJavaIOFileDescriptorAccess(), but this one would also require a JVM option --add-exports java.base/jdk.internal.access=ALL-UNNAMED, so there's not much benefit.

This currently does not address the ExecTerminalProvider which performs an illegal access as mentioned in #575 (comment). However, this provider should only be used when native libraries are not supported, so using a native library as a work around would not really work either.