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

Incorrect constructor selected #91

Closed
rukai opened this issue Mar 1, 2024 · 3 comments
Closed

Incorrect constructor selected #91

rukai opened this issue Mar 1, 2024 · 3 comments

Comments

@rukai
Copy link
Contributor

rukai commented Mar 1, 2024

I am using j4rs to test the java kafka driver against my application.

I have the following code:

use j4rs::{InvocationArg, Jvm, JvmBuilder, MavenArtifact};

fn main() {
    let jvm: Jvm = JvmBuilder::new().build().unwrap();

    jvm.deploy_artifact(&MavenArtifact::from("org.apache.kafka:kafka-clients:3.7.0"))
        .unwrap();
    jvm.deploy_artifact(&MavenArtifact::from("org.slf4j:slf4j-api:1.7.36"))
        .unwrap();
    jvm.deploy_artifact(&MavenArtifact::from("org.slf4j:slf4j-simple:1.7.36"))
        .unwrap();

    jvm.create_instance(
        "org.apache.kafka.clients.admin.NewTopic",
        &[
            InvocationArg::try_from("partitions1").unwrap(),
            InvocationArg::try_from(1_i32)
                .unwrap()
                .into_primitive()
                .unwrap(),
            InvocationArg::try_from(1_i16)
                .unwrap()
                .into_primitive()
                .unwrap(),
        ],
    )
    .unwrap();

    println!("Success");
}

It creates a NewTopic as documented here: https://docs.confluent.io/platform/current/clients/javadocs/javadoc/org/apache/kafka/clients/admin/NewTopic.html
And the source code is here: https://github.com/a0x8o/kafka/blob/master/clients/src/main/java/org/apache/kafka/clients/admin/NewTopic.java

The correct constructor here is NewTopic(String name, int numPartitions, short replicationFactor).
In my test this constructor will often be selected.
However sometimes it will instead select the constructor: NewTopic(String name, Optional<Integer> numPartitions, Optional<Short> replicationFactor)

In the code snippet I pasted, it seems to always select the incorrect constructor, but given the non-deterministic nature of the problem it might behave differently on your machine.

When the code snippet is run I get the following error:

Exception in thread "main" org.astonbitecode.j4rs.errors.InstantiationException: Cannot create instance of org.apache.kafka.clients.admin.NewTopic
	at org.astonbitecode.j4rs.api.instantiation.NativeInstantiationImpl.instantiate(NativeInstantiationImpl.java:47)
Caused by: java.lang.IllegalArgumentException: argument type mismatch
	at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(Unknown Source)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Unknown Source)
	at java.base/java.lang.reflect.Constructor.newInstance(Unknown Source)
	at org.astonbitecode.j4rs.api.instantiation.NativeInstantiationImpl.createInstance(NativeInstantiationImpl.java:101)
	at org.astonbitecode.j4rs.api.instantiation.NativeInstantiationImpl.instantiate(NativeInstantiationImpl.java:44)
Caused by: java.lang.ClassCastException: Cannot cast java.lang.Short to java.util.Optional
	at java.base/java.lang.Class.cast(Unknown Source)
	... 5 more
thread 'main' panicked at src/main.rs:27:6:
called `Result::unwrap()` on an `Err` value: JavaError("An Exception was thrown by Java... Please check the logs or the console.")
stack backtrace:
   0: rust_begin_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/result.rs:1649:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/result.rs:1073:23
   4: blah::main
             at ./main.rs:13:5
   5: core::ops::function::FnOnce::call_once
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@rukai
Copy link
Contributor Author

rukai commented Mar 1, 2024

if (typ instanceof ParameterizedType || typ instanceof WildcardType) {
// For generic parameters, the type erasure makes the parameter be an
// Object.class
// Therefore, the argument is always matched
matchedParams.add(true);

The issue seems to be here, any ParameterizedType in a constructor will always match. This must be causing the Optional to always match. The non-deterministic part of the problem is probably because the list of constructors can be in any order.

I think the solution might be to call some of the methods here https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/ParameterizedType.html in order to get at the types making up the ParameterizedType.
But I'm not really a java person.

@astonbitecode
Copy link
Owner

Indeed, there was an issue when handling the parameterized types to match constructors.
I pushed a fix.
Can you please check that it works for you?

Thanks.

@rukai
Copy link
Contributor Author

rukai commented Mar 4, 2024

I can confirm the example I gave works now.
I ran it in a loop 100 times for good measure.

Thanks for the quick resolution!

@rukai rukai closed this as completed Mar 4, 2024
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

No branches or pull requests

2 participants