GenerateSqlObject in graalvm got NullPointerException

[esotericman]

I use jdbi with version 3.40.0,As the document says:

---

Jdbi provides an annotation processor that creates java implementations of extension types. In addition, it provides an extension factory which does not use proxy objects and returns these as extension type implementations. Calling a method on the extension type will invoke the methods on the generated classes directly.

Here is the generated code

public class ShortUrlRepositoryImpl implements ShortUrlRepository, SqlObject {
    private static final Method m_insert_0 = JdbiClassUtils.methodLookup(ShortUrlRepository.class, "insert", new Class[]{String.class, String.class});
    private static final Method m_selectAll_1 = JdbiClassUtils.methodLookup(ShortUrlRepository.class, "selectAll", new Class[0]);
    private static final Method m_getHandle_2 = JdbiClassUtils.methodLookup(SqlObject.class, "getHandle", new Class[0]);
    private static final Method m_withHandle_3 = JdbiClassUtils.methodLookup(SqlObject.class, "withHandle", new Class[]{HandleCallback.class});
    private final ExtensionMetadata.ExtensionHandlerInvoker i_insert_0;
    private final ExtensionMetadata.ExtensionHandlerInvoker i_selectAll_1;
    private final ExtensionMetadata.ExtensionHandlerInvoker i_getHandle_2;
    private final ExtensionMetadata.ExtensionHandlerInvoker i_withHandle_3;

    public ShortUrlRepositoryImpl(ExtensionMetadata extensionMetadata, HandleSupplier handleSupplier, ConfigRegistry config) {
        this.i_insert_0 = extensionMetadata.createExtensionHandlerInvoker(this, m_insert_0, handleSupplier, config);
        this.i_selectAll_1 = extensionMetadata.createExtensionHandlerInvoker(this, m_selectAll_1, handleSupplier, config);
        this.i_getHandle_2 = extensionMetadata.createExtensionHandlerInvoker(this, m_getHandle_2, handleSupplier, config);
        this.i_withHandle_3 = extensionMetadata.createExtensionHandlerInvoker(this, m_withHandle_3, handleSupplier, config);
    }

    public void insert(String shortUrl, String longUrl) {
        this.i_insert_0.invoke(new Object[]{shortUrl, longUrl});
    }

    public List<ShortUrl> selectAll() {
        return (List)this.i_selectAll_1.invoke(new Object[0]);
    }

    public Handle getHandle() {
        return (Handle)this.i_getHandle_2.invoke(new Object[0]);
    }

    public <R, X extends Exception> R withHandle(HandleCallback<R, X> callback) throws X {
        return this.i_withHandle_3.invoke(new Object[]{callback});
    }

    public static class OnDemand implements ShortUrlRepository, SqlObject {
        private final Jdbi jdbi;

        public OnDemand(Jdbi jdbi) {
            this.jdbi = jdbi;
        }

        public void insert(String shortUrl, String longUrl) {
            this.jdbi.useExtension(ShortUrlRepository.class, (e) -> {
                ((ShortUrlRepositoryImpl)e).insert(shortUrl, longUrl);
            });
        }

        public List<ShortUrl> selectAll() {
            return (List)this.jdbi.withExtension(ShortUrlRepository.class, (e) -> {
                return ((ShortUrlRepositoryImpl)e).selectAll();
            });
        }

        public Handle getHandle() {
            return (Handle)this.jdbi.withExtension(ShortUrlRepository.class, (e) -> {
                return ((ShortUrlRepositoryImpl)e).getHandle();
            });
        }

        public <R, X extends Exception> R withHandle(HandleCallback<R, X> callback) throws X {
            return this.jdbi.withExtension(ShortUrlRepository.class, (e) -> {
                return ((ShortUrlRepositoryImpl)e).withHandle(callback);
            });
        }
    }
}

While executing like this

ShortUrlRepository shortUrlRepository = transactionHandle.attach(ShortUrlRepository.class);
shortUrlRepository.insert("123", "456"); // exception here

error occurs

java.lang.NullPointerException: null
        at org.jdbi.v3.core.extension.ExtensionMetadata$ExtensionHandlerInvoker.lambda$invoke$0(ExtensionMetadata.java:345)
        at org.jdbi.v3.core.AbstractHandleSupplier.invokeInContext(AbstractHandleSupplier.java:36)
        at org.jdbi.v3.core.extension.ExtensionMetadata$ExtensionHandlerInvoker.call(ExtensionMetadata.java:363)
        at org.jdbi.v3.core.extension.ExtensionMetadata$ExtensionHandlerInvoker.invoke(ExtensionMetadata.java:346)
        at org.flmelody.dao.ShortUrlRepositoryImpl.insert(ShortUrlRepositoryImpl.java:45)

With java -jar file ,It works without any problems.May I missed special settings?

[stevenschlansker]

Hi @esotericman , no, there should not be special settings needed to make this work. This looks like a bug either in Jdbi or in Graal (probably jdbi until indicated otherwise). What version of Graal do you use? Is it easy to set up a self contained test case to show the problem?

I have prepared a branch with additional diagnostics:
https://github.com/jdbi/jdbi/tree/diagnose-extensionmethod-npe

Could you run your test against a jdbi snapshot with these changes, and report back with the error message?

[esotericman]

Thanks @stevenschlansker , I tried your snapshot.
It seems that the dao class and its implementation must be added in reflect-config.json.
In my case they're ShortUrlRepository and ShortUrlRepositoryImpl,So I added them like below

  {
    "name":"org.flmelody.dao.ShortUrlRepository",
    "queryAllPublicConstructors":true,
    "queryAllDeclaredMethods" : true,
    "queryAllPublicMethods" : true,
    "methods":[{"name":"<init>","parameterTypes":[] }]
  },
  {
    "name":"org.flmelody.dao.ShortUrlRepositoryImpl",
    "queryAllPublicConstructors":true,
    "queryAllDeclaredMethods" : true,
    "queryAllPublicMethods" : true,
    "methods":[{"name":"<init>","parameterTypes":["org.jdbi.v3.core.extension.ExtensionMetadata","org.jdbi.v3.core.extension.HandleSupplier","org.jdbi.v3.core.config.ConfigRegistry"] }]
  }

And another method return a List about ShortUrl, I also add this

  {
    "name":"org.flmelody.model.ShortUrl",
    "methods":[{"name":"<init>","parameterTypes":["java.lang.Integer","java.lang.String","java.lang.String"] }]
  }

In addition, This Class is also needed

  {
    "name":"org.jdbi.v3.sqlobject.config.internal.RegisterConstructorMapperImpl",
    "methods":[{"name":"<init>","parameterTypes":["java.lang.annotation.Annotation"] }]
  }

After add those lines ,It works now. But a bit complex if there are too many repositories

[stevenschlansker]

Ok, so this sounds similar to #2475. Would it work if the SqlObject generator that writes the implementation class, also writes out a reflect-config.json?

[esotericman]

Yes

[stevenschlansker]

We can try to implement this, but have to fix 2475 first.

[hgschmie]

The point of the generator is to not require this. If it does, I broke something during the refactoring and it needs to be fixed.

Would it be possible to slim down the reflect-config.json above? It would be great to know exactly what is needed. It looks to me it might some sort of constructor lookup.

[hgschmie]

I see two problems:

• we derive the class for the implementation and ondemand types from the extension type when loading the extension by using the extension type name, computing the derived name (XXXImpl or XXX$OnDemand) and then call Class.forName. GraalVM can intercept those and if the parameters are a constant, can replace them. They are not constants for us.
• we then do some c'tor magic to find the right constructor. This is actually not needed in this case, just the method was convenient. So the getDeclaredConstructor method only takes constants as parameters with the exception of the Class itself.

Changing getGeneratedClass and getOnDemandClass in GeneratorSqlObjectFactory to this:

private static MethodHandleHolder<?> getGeneratedClass(Class<?> extensionType) {

        try {
            var generatedClass = Class.forName(getGeneratedClassName(extensionType));  // dynamic lookup of implementation class
            var classConstructor = generatedClass.getDeclaredConstructor(EXTENSION_TYPES); // create instance using the class found.
            var methodHandle = MethodHandles.lookup().unreflectConstructor(classConstructor);
            var constructorHandle = methodHandle.asType(methodHandle.type().changeReturnType(Object.class));

            return invoker -> {
                try {
                    return invoker.createInstance(constructorHandle);
                } catch (Throwable t) {
                    throw throwAnyway(t);
                }
            };
        } catch (Throwable t) {
            throw new UnableToCreateSqlObjectException(t);
        }
    }

    private static MethodHandleHolder<?> getOnDemandClass(Class<?> extensionType) {
        try {
            var generatedClass = Class.forName(getOnDemandClassName(extensionType));
            var classConstructor = generatedClass.getDeclaredConstructor(ON_DEMAND_TYPES);
            var methodHandle = MethodHandles.lookup().unreflectConstructor(classConstructor);
            var constructorHandle = methodHandle.asType(methodHandle.type().changeReturnType(Object.class));

            return invoker -> {
                try {
                    return invoker.createInstance(constructorHandle);
                } catch (Throwable t) {
                    throw throwAnyway(t);
                }
            };
        } catch (Throwable t) {
            throw new UnableToCreateSqlObjectException(t);
        }
    }

The first two lines in each method are most likely the problem. If you have a test project that works running under the regular JVM and crashes when compiled natively, I would be interested to try that out. Maybe we can pin this down further.

[stevenschlansker]

If the reflective lookup of classes with non-constant names is a problem, we could evaluate other ways of discovering these like a ServiceLoader entry.

[hgschmie]

that may work but it creates a lot of ceremony. Users now have to create a serviceloader entry with the right file name (which is currently hidden away from them) and they also need to add this in the future to their module-info.java

[hgschmie]

so spending a bit of time on this issue, it seems that this is a bit more fundamental. Any native app must declare all of its pieces where it needs reflection so that it ultimately creates a reflection-config.json.

e.g. in the example above, there are notes needed for the dao, its implementation but also for the data object itself and when we are using some of the mappers.

Other systems such as quarkus use an annotation for that and have a build step processor that collects the information from the build and writes the config files. see https://quarkus.io/guides/writing-native-applications-tips for an example.

I don't think that any generic "this is how we declare reflection needed for native" framework exists yet. We could roll our own annotation and then provide meta-annotation processors e.g. for quarkus, spring-aot or other frameworks.

as dropwizard as one of the primary consumers of Jdbi has no real ambition to move to native (dropwizard/dropwizard#2974), we may be stuck rolling our own thing for now and provide glue.

[stevenschlansker]

I was of course imagining the annotation processor would generate the necessary service loader configuration. Or, it could generate the needed reflection-config.json. No need for a generic solution; we're already writing a code generator.