C library for Pkl

[KushalP]

This introduces native C bindings for Pkl.

Dynamic Library

Using org.graalvm.nativeimage and the native-image binary we
produce a dynamic library (for each OS/Arch variant) that provides a
way to initialise itself with a graal_isolatethread_t.

Methods are annotated with @CEntryPoint and exported.

This change results in an architecture and OS-specific directory being
created which now produces the headers for our shared library
functions:

❯ ll libpkl/build/libs/macos-aarch64/

graal_isolate_dynamic.h
graal_isolate.h
libpkl_internal_dynamic.h
libpkl_internal.dylib
libpkl_internal.h

libpkl

The produced libpkl dynamic library wraps the GraalVM C interface
into something that is future-friendly for the needs of a Pkl
integrator. It exports an interface which aligns with SPICE-0015[1].

❯ ll libpkl/build/libs/macos-aarch64/

graal_isolate_dynamic.h
graal_isolate.h
libpkl_internal_dynamic.h
libpkl_internal.dylib
libpkl_internal.h
libpkl.dylib    <--- this is new
pkl.h           <--- this is new

JNA

Testing of the produced libpkl dynamic library is done using Java
Native Access[2] for ease. We provide an interface in Kotlin which
JNA transposes against the libpkl dynamic library discoverable at
the path that is discoverable with jna.library.path.

Load in projects.pklCommonsCli to deal with UnsupportedFeatureException

This is to deal with the following error:

Caused by: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Detected a started Thread in the image heap. Thread name: main. Threads running in the image generator are no longer running at image runtime. If these objects should not be stored in the image heap, you can use

    '--trace-object-instantiation=java.lang.Thread'

[1] apple/pkl-evolution#16

[KushalP]

Does this work? What's the preferred approach to take here?

[bioball]

You should be able to test this locally (set the env var, run unit tests and see what happens)

[bioball]

Great work so far! I left some comments, but it's great to see this already working.

[bioball]

Can you add doxygen-style doc comments to these methods?

Also: let's add a void pointer here (per our convos with some C folks); we'd just keep a reference to this pointer and pass it back when calling PklMessageResponseHandler.

Suggested change

	typedef void (*PklMessageResponseHandler)(int length, char* message);
	int pkl_init(PklMessageResponseHandler handler);
	int pkl_send_message(int length, char* message);
	int pkl_close();
	typedef void (*PklMessageResponseHandler)(int length, char *message, void *handlerContext);
	int pkl_init(PklMessageResponseHandler handler, void *handlerContext);
	int pkl_send_message(int length, char *message);
	int pkl_close();

BTW: it seems like putting the asterisk next to the name of the variable is more idiomatic in C

[KushalP]

How would you expect handlerContext to be provided back to the user in int pkl_init(PklMessageResponseHandler handler, void *handlerContext); ?

[bioball]

We'd just pass it back every time we call handler, which also accepts void *handlerContext.

[bioball]

We don't need to address this before merging into a feature branch, but, this file should be platform-independent.

I think this should be:

#include <pkl_internal.h>

The platform-specific header/library would then be provided as a linker option.

[KushalP]

This is almost there. I don't know if there's a way to get native-image to produce a dynamic library prefixed with lib whilst the .h file it creates doesn't have the lib prefix. Otherwise we'll have to rename it ourselves.

[bioball]

It's not a big deal if it does have a lib prefix, I guess. That's an internal file anyway; users would import pkl.h and not libpkl_internal.h

[bioball]

Is there a particular reason we are creating our own method, instead of graal_create_isolate from the Native Image C API?

https://www.graalvm.org/latest/reference-manual/native-image/native-code-interoperability/C-API/

I think we can just handle this in our pkl_init:

graal_isolatethread_t *isolatethread = NULL;
graal_isolate_t *isolate = NULL;

int pkl_init(PklMessageResponseHandler handler) {
  // etc
  if (graal_create_isolate(NULL, &isolate, &isolatethread) != 0) {
    return -1
  }

[KushalP]

I thought it would be consistent with how all of the pkl-internal bits are exposed through LibPkl.java. This would now be one case where the management of Graal-specific resources is handled out-of-band in the .c file. If you'd prefer me to do the above let me know.

[bioball]

I'm not sure what the benefit of doing it this way is, especially since there already is a C method that does the same thing.

But, they have a tutorial where they do it this way too: https://www.graalvm.org/21.3/reference-manual/native-image/ImplementingNativeMethodsInJavaWithSVM/

So, I guess let's just keep this? We can ask about this in their Slack too.

[bioball]

We can address this in a follow-up PR, but: I think let's move org.pkl.server.AbstractServerTest into pkl-commons-test.

[bioball]

This class is mixing quite a few concerns.

• Does this need to implement Iterable? I don't see where that's being used
• Should separate the callback into a different object

I think this can just have an init() that calls into ILibPklLibrary:

  private val messageResponseHandler: PklMessageResponseHandler = object : PklMessageResponseHandler {
    override fun invoke(length: Int, message: Pointer) {
      val receivedBytes = message.getByteArray(0, length)
      val message = decode(receivedBytes)
      assertThat(message).isNotNull
      incoming.add(message!!)
    }
  }

  fun init() {
    assertThat(ILibPklLibrary.INSTANCE.pkl_init(messageResponseHandler)).isEqualTo(0)
  }

  override fun close() {
    incoming.clear()
    assertThat(ILibPklLibrary.INSTANCE.pkl_close()).isEqualTo(0)
  }

[KushalP]

Does this need to implement Iterable? I don't see where that's being used

By making it Iterable in JNATest.kt I can write assertions like the following: assertThat(client).hasSize(1).

[bioball]

void *handlerContext should be on pkl_init, not pkl_send_message.