Registered resources are being GC:ed

[richarddd]

Hello,

I've discovered an issue where registered resources can be garbage collected if some time occurs between when they are being registered and Snapshot is taken.

For example:

Core.getGlobalContext().register(new Resource() {
    @Override
    public void beforeCheckpoint(Context<? extends Resource> context) throws Exception {
        System.out.println("beforeCheckpoint() invoked");
    }

    @Override
    public void afterRestore(Context<? extends Resource> context) throws Exception {
        System.out.println("afterCheckpoint() invoked");

    }
});

//try force gc
for (int i = 0; i < 10; i++) {
    System.gc();
}

try {
    Thread.sleep(10000);
} catch (InterruptedException e) {
    throw new RuntimeException(e);
}

//try force gc
for (int i = 0; i < 10; i++) {
    System.gc();
}

My assumption is that the WeakHashMap in ResourceWrapper will release the references since the strongRef is only set in the beforeCheckpoint method

[AntonKozlov]

Hello,

That's indeed correct that before checkpoint CRaC maintains only weak references to the resources. So CRaC itself does not prevent a registered object to be GCed. This was done intentionally to avoid the additional burden of deregistering Resources before losing all explicit links in user code.

In this case, if you want to be sure the Resource is notified, you can create a local variable that will prevent the Resource to be GCed.

In this case, org.crac is consistent with JDKs CRaC API. https://crac.github.io/openjdk-builds/javadoc/api/java.base/jdk/crac/package-summary.html

If you know a better solution that won't also require explicit Resource deregistration, please let me know.

[richarddd]

Hello,

That's indeed correct that before checkpoint CRaC maintains only weak references to the resources. So CRaC itself does not prevent a registered object to be GCed. This was done intentionally to avoid the additional burden of deregistering Resources before losing all explicit links in user code.

In this case, if you want to be sure the Resource is notified, you can create a local variable that will prevent the Resource to be GCed.

In this case, org.crac is consistent with JDKs CRaC API. https://crac.github.io/openjdk-builds/javadoc/api/java.base/jdk/crac/package-summary.html

If you know a better solution that won't also require explicit Resource deregistration, please let me know.

Hey Anton, thanks for your reply.

I understand the concept but i think the current behavior is a bit risky as it "might" work or not work depending on what you do in between your registration and your checkpointing.

This can introduce issues where users will have a hard time finding the root cause as the behavior is nondeterministic. For example, it might work fine in tests where lots of dependencies are mocked but in a production environment a GC sweep might have cleared the resource.

I recon this will occur mostly if we register anonymous classes. We can force only anonymous classes to always be strong in the ResourceWrapper, but that doesn't solve the deregistration you want to avoid.

Shouldn't deregistration be offered tho? Doesn't it offer more control versus weak references?
If i compare this to for example JavaScript event listeners in the browser: Regardless if they are anonymous functions or strong references it's up to the user to register/unregister them and thus the behavior is deterministic.

[smirnoal]

I suggest to update Context.register() javadoc a bit to explicitly mention that Context only holds a WeakRefernce. This doc already tells the same, but probably not as visible in an IDE

[AntonKozlov]

I recon this will occur mostly if we register anonymous classes. We can force only anonymous classes to always be strong in the ResourceWrapper, but that doesn't solve the deregistration you want to avoid.

The same problem will be with a named Resource implementation that is GCed

Shouldn't deregistration be offered tho? Doesn't it offer more control versus weak references? If i compare this to for example JavaScript event listeners in the browser: Regardless if they are anonymous functions or strong references it's up to the user to register/unregister them and thus the behavior is deterministic.

Our API is designed for an object that is meaningful by its own and has a state that needs to be managed on checkpoint and restore. By creating weak references we avoid any changes in the lifecycle by default.

An anonymous resource implementation (or a java lambda, or a callback in javascript) is a meaningless object by its own, and that is just a container for a standalone code. For example, in AWT it is possible to register an event listener that glues different components. The object needs not be GCed, so a strong reference is created by default. Deregistering is not that frequent, as most objects are created just to be registered.

And we can build strong links on top of weak ones, by just adding a strong link that will prevent GC to clean the object. For usability, it can be another context. For example, https://gist.github.com/AntonKozlov/e24754caa1a850d35d246faabaca845f#file-testweak-java-L9 provides a strongly registering "view" for the global context. With some effort, it should be possible to provide weak references on top of strong ones, but what is more useful is still a question.

Both approaches are possible, but I think conceptually weak refs are more natural in our case. Are there other examples of when a strong link is more natural?