Skip to content
This repository has been archived by the owner on Nov 14, 2023. It is now read-only.

Wrong module descriptor in org.lwjgl #10

Closed
io7m opened this issue Dec 28, 2018 · 14 comments
Closed

Wrong module descriptor in org.lwjgl #10

io7m opened this issue Dec 28, 2018 · 14 comments
Assignees
Labels

Comments

@io7m
Copy link
Collaborator

io7m commented Dec 28, 2018

The process used to create the modules is putting the wrong module descriptor in the org.lwjgl.lwjgl jar:

$ unzip -p org.lwjgl.lwjgl-3.2.1.jar META-INF/versions/9/module-info.class | strings
SourceFile
module-info.java
Module
10.0.2
module-info
org.lwjgl.jemalloc.natives
	java.base
org.lwjgl.jemalloc

We might need to insert a custom module descriptor.

@io7m io7m added the bug label Dec 28, 2018
@io7m io7m self-assigned this Dec 28, 2018
@io7m
Copy link
Collaborator Author

io7m commented Dec 28, 2018

Basically, for future reference: We had to merge lwjgl and lwjgl-jemalloc due to an (implicit) circular dependency. The lwjgl code does this:

final class MemoryManage {

    private MemoryManage() {
    }

    static MemoryAllocator getInstance() {
        Object allocator = Configuration.MEMORY_ALLOCATOR.get();
        if (allocator instanceof MemoryAllocator) {
            return (MemoryAllocator)allocator;
        }

        if (!"system".equals(allocator)) {
            String className;
            if (allocator == null || "jemalloc".equals(allocator)) {
                className = "org.lwjgl.system.jemalloc.JEmallocAllocator";
            } else if ("rpmalloc".equals(allocator)) {
                className = "org.lwjgl.system.rpmalloc.RPmallocAllocator";
            } else {
                className = allocator.toString();
            }

            try {
                Class<?> allocatorClass = Class.forName(className);
                return (MemoryAllocator)allocatorClass.getConstructor().newInstance();
            } catch (Throwable t) {
                if (Checks.DEBUG && allocator != null) {
                    t.printStackTrace(DEBUG_STREAM);
                }
                apiLog(String.format("Warning: Failed to instantiate memory allocator: %s. Using the system default.", className));
            }
        }

        return new StdlibAllocator();
    }

In other words, lwjgl implicitly depends on lwjgl-jemalloc, and lwjgl-jemalloc has hard dependencies on bits of the org.lwjgl.system package. The two had to be combined in order to allow lwjgl to work.

@io7m
Copy link
Collaborator Author

io7m commented Dec 28, 2018

The custom module descriptor needs to look like this:

/*
 * Copyright LWJGL. All rights reserved.
 * License terms: https://www.lwjgl.org/license
 */
module org.lwjgl {
    requires transitive jdk.unsupported;

    exports org.lwjgl;
    exports org.lwjgl.system;
    exports org.lwjgl.system.dyncall;
    exports org.lwjgl.system.jemalloc;  // Added for OSGi bundles
    exports org.lwjgl.system.jni;
    exports org.lwjgl.system.libc;
    exports org.lwjgl.system.linux;
    exports org.lwjgl.system.macosx;
    exports org.lwjgl.system.windows;
}

@Spasi
Copy link
Member

Spasi commented Dec 28, 2018

I don't get this. The JEmallocAllocator is loaded via reflection and if jemalloc isn't in the class/module-path, there's a fallback that's always available. The org.lwjgl does not contain the org.lwjgl.system.jemalloc package, so I'm not sure what exports org.lwjgl.system.jemalloc; is supposed to achieve. What happens when both org.lwjgl and org.lwjgl.jemalloc modules are present?

@io7m
Copy link
Collaborator Author

io7m commented Dec 28, 2018

I don't get this. The JEmallocAllocator is loaded via reflection and if jemalloc isn't in the class/module-path, there's a fallback that's always available.

In OSGi, there isn't a classpath or a module path. The OSGi container wires individual packages from other modules/bundles based on Import-Package and Export-Package declarations. The attempt to call Class.forName won't work on OSGi because lwjgl doesn't import jemalloc (and it can't, because that would create a circular dependency between lwjgl and jemalloc). In order to get around this, we combined the lwjgl and jemalloc modules.

What is the fallback allocator? Has it always been there? The reason I merged the two modules in the first place was because it seemed like jemalloc was required for the system to work at all. This was way back at version 3.1.3 though...

@io7m
Copy link
Collaborator Author

io7m commented Dec 28, 2018

To be clear: The arrangement we're using works fine for OSGi right now, it's just that I'd like the OSGi bundles to continue to work as JPMS modules and I've just discovered today that I've accidentally broken the one module above. It's possible that I could avoid exporting the jemalloc package... It's not clear to me if anything outside of the lwjgl core would talk to it directly. If I could avoid exporting that package, then I could use the original lwjgl module descriptor unmodified (which would be preferable!).

@io7m
Copy link
Collaborator Author

io7m commented Dec 28, 2018

The attempt to call Class.forName won't work on OSGi because lwjgl doesn't import jemalloc (and it can't, because that would create a circular dependency between lwjgl and jemalloc).

Sorry, reading that back, I realized I explained it rather poorly.

OSGi uses a peer-to-peer (non-hierarchical) classloading model at package granularity. A bundle states which packages it exports, and which packages it imports. A bundle implicitly imports all of the packages that it exports. In order for a bundle A to see a class C in a package P that's exported by a bundle B, there must be an Import-Package directive in A that imports P. At runtime, the OSGi system will wire bundle A to bundle B to satisfy the import of package P (assuming that there isn't some other bundle that exports a more appropriate version of P with respect to version constraints and the like).

A bundle can always see classes in its own packages, whether it exports those packages or not.

Given the above explanation, we can assume that Class.forNamewill work fine if the calling code lives in a bundle that happens to have an Import-Package directive for the package that contains the named class. Class.forName will also work correctly if it refers to a class that's in the same bundle as the calling code.

We can't put in the appropriate Import-Package directives in the lwjgl and jemalloc bundles in this case, because it would cause a circular dependency. We can however merge the two bundles; this allows the Class.forName call shown above to work. That's what we did all that time ago, but maybe now it's possible to unmerge them?

@io7m io7m closed this as completed in f8e42d5 Dec 29, 2018
@Spasi
Copy link
Member

Spasi commented Dec 29, 2018

What is the fallback allocator? Has it always been there?

Yes, it's the default system memory allocator (also accessible via org.lwjgl.system.libc.LibCStdlib). Jemalloc is only used if instantiating org.lwjgl.system.jemalloc.JEmallocAllocator succeeds, it's completely optional.

It's not clear to me if anything outside of the lwjgl core would talk to it directly.

It's very likely. Applications can use org.lwjgl.system.jemalloc.JEmalloc directly in advanced memory management scenarios (tracking memory usage, using dedicated arenas for certain resources, etc). JEmalloc also exposes non-standard API that's not available via MemoryUtil.

OSGi uses a peer-to-peer...

How does OSGi do optional (i.e. equivalent to requires static <module>;) dependencies? How does it do service discovery (i.e. equivalent to uses and provides ... with ...)? The org.lwjgl module does not depend on any class in the org.lwjgl.jemalloc module. It only depends on org.lwjgl.system.MemoryUtil$MemoryAllocator (which already belongs to org.lwjgl), for which org.lwjgl.jemalloc provides an implementation.

Also, what about rpmalloc? The solution for jemalloc should be equally applicable to rpmalloc, it can optionally be used in much the same way (just not by default).

@io7m
Copy link
Collaborator Author

io7m commented Dec 29, 2018

Yes, it's the default system memory allocator (also accessible via org.lwjgl.system.libc.LibCStdlib). Jemalloc is only used if instantiating org.lwjgl.system.jemalloc.JEmallocAllocator succeeds, it's completely optional.

Hm... Not sure why I thought it was required, then. I'll try separating the two bundles again and see what happens.

It's very likely. Applications can use org.lwjgl.system.jemalloc.JEmalloc directly in advanced memory management scenarios (tracking memory usage, using dedicated arenas for certain resources, etc). JEmalloc also exposes non-standard API that's not available via MemoryUtil.

Hm, right.

How does OSGi do optional (i.e. equivalent to requires static ;) dependencies?

The Import-Package directive can take a resolution argument that can be set to optional:

Import-Package: x.y.z;resolution=optional

... However this as seen as somewhat poor style, because the general opinion is that services should be used to express code that may be optional at runtime. The resolution parameter isn't capable of breaking a circular dependency either - the optional part should just be read as "do not fail resolution if this particular package isn't present".

How does it do service discovery (i.e. equivalent to uses and provides ... with ...)?

There are multiple ways to achieve this in OSGi. OSGi treats services the same way as JPMS services; a service is just an ordinary Java class or interface and you can register (and deregister) instances of a service programatically or declaratively ("declarative services") by annotating classes that are then processed by the various OSGi tools (like Bnd). OSGi best practice is currently to avoid depending on the OSGi APIs directly unless absolutely necessary. OSGi people are pretty service-obsessed; it's a core part of the specification.

There's also an optional module in the OSGi compendium specification that describes a component that makes it possible to use code that uses ServiceLoader in an OSGi container. I've not personally tried this one. The main implementation of this is Apache Aries SPI Fly. You can install it into any OSGi container. I'm told it's good, but I've not personally tried it.

It is possible to write code that can work as both a JPMS and OSGi service. In my own projects, I tend to abstract over the part that looks up services. Let's say I've got a bit of code that wants to look through a set of available parser implementations, and it wants to do this whilst remaining ignorant of whether it's running on OSGi or JPMS. In this case, we model parser implementations as services. I'd first define:

interface ParserRegistryType
{
   List<ParserType> availableParsers();
}

Then I'd implement the registry once for JPMS:

class ParserRegistryJPMS implements ParserRegistryType
{
    @Override
    List<ParserType> availableParsers()
    {
        final var loader = ServiceLoader.load(ParserType.class);
        // iterate over and collect implementations...
    }
}

Then I'd implement one for OSGi (using compile-time-only declarative services annotations):

@Component
class ParserRegistryOSGi implements ParserRegistryType
{
    private Set<ParserType> parsers = new ConcurrentHashMap().newKeySet();

    // The @Reference annotation takes various arguments that can describe
    // service cardinality, whether a service is required or not, etc...
    @Reference(...)
    public void onParserAvailable(ParserType parser)
   {
      parsers.add(parser);
   }

    @Override
    List<ParserType> availableParsers()
    {
       return parsers;
    }
}

Then I'd write my original class that wanted to look at parser implementations such that it required the user to pass in a value of type ParserRegistryType. If the user is on OSGi, they'd pass in a reference to ParserRegistryOSGi. Otherwise, they'd use ParserRegistryJPMS. The code could also default to using ParserRegistryJPMS unless the user says otherwise (I often add some static factory methods that do this).

@io7m io7m reopened this Dec 29, 2018
@io7m
Copy link
Collaborator Author

io7m commented Dec 29, 2018

I forgot to mention that the analogues to uses and provides in OSGi are the Provide-Capability and Require-Capability directives. These are strictly more general than JPMS uses and provides in that they can express more than just class/type dependencies. I wrote a short blog post on this a while back.

Here are a couple of examples from a library I maintain:

Provide-Capability                       osgi.service;objectClass:List<String>="com.io7m.smfj.parser.api.SMFParserProviderType,com.io7m.smfj.probe.api.SMFVersionProbeProviderType,com.io7m.smfj.serializer.api.SMFSerializerProviderType"

The above should be read as "I provide implementations of three services: com.io7m.smfj.parser.api.SMFParserProviderType, com.io7m.smfj.probe.api.SMFVersionProbeProviderType, and com.io7m.smfj.serializer.api.SMFSerializerProviderType".

Require-Capability                       osgi.extender;filter:="(&(osgi.extender=osgi.component)(version>=1.3.0)(!(version>=2.0.0)))",osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=9.0))"

The above should be read as "I require an OSGi container that has an implementation of at least version 1.3.0 of the osgi.component extender installed, and I require an execution environment that supports at least Java 9". The osgi.component extender is the module that allows declarative services to work.

Both of these directives are generated automatically by Bnd. It should essentially never be necessary to write any of this metadata by hand.

io7m added a commit that referenced this issue Dec 29, 2018
This separates the org.lwjgl.lwjgl and org.lwjgl.jemalloc modules
that were joined back in the original OSGi release.

Affects #10
@io7m
Copy link
Collaborator Author

io7m commented Dec 29, 2018

I believe I merged jemalloc and lwjgl originally because doing otherwise seemed to cause some sort of exception at runtime. For whatever reason, that no longer happens - the modules are now separated again!

Unfortunately, it does mean that you still can't override the default allocator to be jemalloc or rpmalloc on OSGi, but maybe we should deal with that when someone actually asks for it. 😄

Anyone needing the extended jemalloc interface can get it if they need it.

@io7m
Copy link
Collaborator Author

io7m commented Dec 29, 2018

@Spasi If you've no objection, I'll push a 3.2.1.1 release with the new jemalloc module and the correct module descriptors from #11

@Spasi
Copy link
Member

Spasi commented Dec 29, 2018

Sure, go ahead.

Just to be clear, what happens now when an OSGi application includes jemalloc? Using jemalloc directly works, but the Class.forName call in org.lwjgl fails and MemoryUtil always uses the default memory allocator?

@io7m
Copy link
Collaborator Author

io7m commented Dec 29, 2018

Using jemalloc directly works, but the Class.forName call in org.lwjgl fails and MemoryUtil always uses the default memory allocator?

Yep, that's it.

@io7m
Copy link
Collaborator Author

io7m commented Dec 29, 2018

Final thing: Manually setting Configuration.MEMORY_ALLOCATOR also works, because the caller is responsible for doing the class resolution. I've put this in the README.

@io7m io7m closed this as completed Dec 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants