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

Access to default methods in @JImplements #1182

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Thrameos
Copy link
Contributor

Default methods are a pain because the proxy to interfaces always redirects and there is no "normal" method of accessing such a method via reflection. The PR uses MethodHandles to perform the redirect. It is no clear if this works in Java 8 though as some of the comments regarding this method indicate that it may be a Java 9 feature. I added test cases to see if this is true. If it is only a Java 9 feature we can try to implement a mixed mode jar file, but that will get more complicated.

Fixes #1181

@Thrameos
Copy link
Contributor Author

Will be hard to get this to work as the jar file must be loaded into the base classloader. It may be doable, but will require serious effort.

@marscher
Copy link
Member

marscher commented May 7, 2024

sorry I cannot estimate if it'd be worth the effort? How common are these "default methods"?

@Thrameos
Copy link
Contributor Author

Thrameos commented May 7, 2024

Default methods are a newer feature of Java in which the programmers adds some functionality to an interface which will automatically be available to the implemented classes. They are common in newer code, but most of the Java library classes currently don't use them. They are difficult to access because they didn't exist when JNI was written so there is no "call the default" method and the proxy system works against them. The issue here is that the newer java security methods prevent calling code from arbitrary modules, and our jar is not located in a position of any privilege as we are not in the boot loader.

About the only way I can make this happen would be to make my own Jar loader which uses the JNI special under the table class definition method which in theory would get the JPype into a lower level. However, JNI doesn't really have any support for modules or module definition so I have no idea if I can actually get to the level required to make this type of access. Making a jar side loader would require access to gzip and tar access. There may be severe technical issues if I need to load the class files in a particular order.

One can expect that many libraries will have default method if they use interfaces at some point. Thus there is some value, but it is a large lift (and given I am underwater until July it won't happen soon.)

@marscher
Copy link
Member

marscher commented May 8, 2024

Bypassing JVMs security mechanisms does not seem right. If these extended Interfaces do not posses a mechanism in JNI, we should request them upstream. It does not make any sense for them to be inacessible, right?

Why are default methods not already handled by the MRO, e.g. calling the parents implementation? Actually it should, should it not?
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8033126

@Thrameos
Copy link
Contributor Author

Thrameos commented May 8, 2024 via email

@Thrameos
Copy link
Contributor Author

I am still trying to come up with a solution for this. The Java9 security model is the source of the issues. Certain actions require that the code be loaded into the lowest layer of the JVM and be contained in an authorized module. There was no thought as to how the JNI layer interacts as it doesn't have a module nor is formally loaded into the ClassLoader other than by class which invoked it first.

A lot of stuff we have yet to be able to support such as calling default methods, calling protected methods/accessing protected members when extending a class are all stuck on this same common element. Without appropriate privileges all calls end up as second class and are denied.

I have tried a number of different ways to work around this:

  1. Force the org.jpype.jar into the classpath.
    Doable, but argues with the user specified classpath. This is mostly a problem of the grandfathered and should have been deprecated "-Dclasspath=XXX" startJVM option. As we have to massage the classpath the user provided there are a lot of edge cases. As best, I can make the startJVM through and error on manually setting the classpath, but I am sure that will break a lot of old code.

  2. Try to install the jar classes under the JVM using JNI DefineClass. This is doable but runs into clear problems as some classes use resources which don't exist when loaded that method. And second all hell breaks lose if the jar is loaded through both the dynamic loader and the define class method for some reason. I have thus abandoned this.

  3. Use an agent to allow "late" loading of classes in the class path. This method would use Instrumentation API to slip jar files in after the JVM is started. The "org.jpype" jar would just use this path to install itself and we can insert other jars so that multiple calls to startJVM would to the same. Again the problem here is conflict. To use an agent I would have ship a second jar file org.jpype.agent.jar which grants us the superpowers we need. But there are times (rare) that a person actually installs an agent through startJVM and in those cases we will have conflicts. I can give agent that same treatment that classpath got where the user is not setting things directly but through an API which can resolve conflicts.

I did some testing on the javaagent option. As far as I can tell noone outside of my group makes use of agents in the startJVM() (at least no google searches show an intersection). The only time that it gets used in my group is when doing certain profiling tasks.

Looking deeper I think that agent may solve the problem though in a weird way. You can start multiple agents with the repeated arguments so it isn't like java.class.path. Second it appears that if we simply make org.jpype.jar an agent (even if it doesn't do anything) it will automatically be promoted to a level were we can access reflection freely.

The problem with the JVM classpath security model is that assumes that everything must be known at start of JVM and thus the only jars that can be trusted exist at that time. Many jar types that provide services just don't work after the JVM is started. I have not found any API which allows the JNI controlling process which spawned the JVM to control the access privileges. The entire model of the JVM is "one and done" for any and all settings.

I am guessing that is either laziness on the part of JVM developers (this is edge case stuff and we have better things to do), neglect (which JNI is clearly suffering from), or some lame attempt at security from back when the JVM was still running in browsers and people were sandbox breaking. Regardless of the case, it gives us the classic "sucks to be you" feeling as seeming reasonable requests just can't be met. It makes me long for that far future in which graalvm or web assembly VM simply provides a language independent platform which Python and Java can play nicely.

@Thrameos
Copy link
Contributor Author

@marscher I think this one may be working now. But we need other PR to get test matrix back on line first.

@Thrameos Thrameos requested a review from marscher June 24, 2024 16:50
Copy link
Contributor Author

@Thrameos Thrameos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this PR is a lot to chew on, I have highlighted the critical sections.

Comment on lines +240 to +244
supportLib = os.path.join(os.path.dirname(os.path.dirname(__file__)),"org.jpype.jar")
if not os.path.exists(supportLib):
raise RuntimeError("Unable to find org.jpype.jar support library at "+supportLib)
extra_jvm_args += ('-javaagent:'+supportLib,)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code changes the way that the jar gets loaded to place in a privileged enough position to call reflect. Prior to Java 9 this was the norm, but after 9 there was a new module based security policy and org.jpype is not organized into a module.

Comment on lines -26 to -42
static jobject toURL(JPJavaFrame &frame, const string& path)
{
// file = new File("org.jpype.jar");
jclass fileClass = frame.FindClass("java/io/File");
jmethodID newFile = frame.GetMethodID(fileClass, "<init>", "(Ljava/lang/String;)V");
jvalue v[3];
v[0].l = frame.NewStringUTF(path.c_str());
jobject file = frame.NewObjectA(fileClass, newFile, v);

// url = file.toURI().toURL();
jmethodID toURI = frame.GetMethodID(fileClass, "toURI", "()Ljava/net/URI;");
jobject uri = frame.CallObjectMethodA(file, toURI, nullptr);
jclass uriClass = frame.GetObjectClass(uri);
jmethodID toURL = frame.GetMethodID(uriClass, "toURL", "()Ljava/net/URL;");
return frame.CallObjectMethodA(uri, toURL, nullptr);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the Jar loaded as an agent we don't need to worry about loading it late in the process so all this code can be removed.

JP_RAISE_METHOD_NOT_FOUND(cname);
return nullptr;
}
return missing;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method to fail is moved to the Java code so the corresponding JNI code to trigger has been removed.

Comment on lines +1 to +11
package org.jpype.agent;

import java.lang.instrument.Instrumentation;

public class JPypeAgent
{
public static void premain(String agentArgs, Instrumentation inst) {
// This doesn't have to do anything.
// We just need to be an agent to load elevated privileges
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We aren't going to do anything with the agent status except use the load path. So we don't need to have a body.

We can use the agent to do things like change byte code or access low level memory statistics for the JVM which may be handy. We can also late load a jar file to the same level which is required for things like DB services.

Comment on lines +104 to +107
return MethodHandles.lookup()
.unreflectSpecial(method, method.getDeclaringClass())
.bindTo(proxy)
.invokeWithArguments(args);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the heart of the change as we need to call these privileged methods to invoke a special method. Of course given the method we are trying to invoke is nothing more than a public method implemented in the interface there is absolutely nothing privileged about the call. The issue is simply that the API through which we are accessing a public method can also be used to invoke private methods.

@@ -35,6 +36,7 @@ public class JPypeProxy implements InvocationHandler
public long cleanup;
Class<?>[] interfaces;
ClassLoader cl = ClassLoader.getSystemClassLoader();
public static Object missing = new Object();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This static object will be passed back to indicate that Python did not implement the method. There is nothing special about it other than being an object that the user is unlikely to ever return from a valid proxy. The object was left public so that I can pass it during testing to trigger the not implemented logic if needed.

Comment on lines +321 to +323
if file.endswith("manifest.txt"):
manifest = file
continue
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another requirement for an agent. We must add contents to the manifest and include it in the jar file.

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.

Clarity in Implementation of interfaces with default methods
2 participants