quarkus-unleash
quarkus-unleash copied to clipboard
Memory leak risk with `@FeatureToggle Instance<Boolean>` and `@FeatureVariant Instance<?>`
From what I saw in the extension code, when @FeatureToggle Instance<Boolean> or @FeatureVariant Instance<?> are used, the injected bean doesn't have an explicit CDI scope which means it defaults to @Dependent.
When a @Dependent bean is injected with Instance<?> into an @ApplicationScoped or @Singleton bean and is no destroyed when it's no longer needed, it stays in the memory forever, so there's basically a memory leak.
Is there a risk of memory leak with this extension, or did I miss something? If there is a risk of leak, the doc should at least explain how to prevent it and show how the injected bean can be destroyed.
@gwenneg thank you for checking the implementation. Could we change the implementation to avoid the memory leak?
If the leak is confirmed (which I'll try to check next week), there are several ways to "fix" it.
One of them would be a simple doc update, showing how to destroy the Boolean field after it's been used:
try (Instance.Handle<Boolean> toggleHandle = toggle.getHandle()) {
boolean toggleValue = toggleHandle.get();
// do something with toggleValue
}
There are other approaches such as using a wrapper class for the toggle field. But before considering any changes, the leak has to be confirmed 😄
Hey @mkouba!
Is there something in Arc that prevents memory leaks when a @Dependent bean is injected into an @ApplicationScoped bean with Instance<?> and not destroyed afterwards?
public class FooProducer {
@Produces
@Dependent
Foo produce() {
return new Foo();
}
}
@ApplicationScoped
public class LongLivedBean {
@Inject
Instance<Foo> foos;
public void doSomething() {
Foo foo = foos.get();
// use foo without calling foos.destroy(foo) eventually
}
}
I expected a potential memory leak in this extension but my profiler showed a healthy memory so I may be missing something... 😅
I expected a potential memory leak in this extension but my profiler showed a healthy memory so I may be missing something... 😅
So both Weld and ArC try to mitigate this problem in a way that only some references to @Dependent beans are stored in the CreationalContext.
In ArC, we only add a @Dependent bean to the CreationalContext if (1) a class bean has @PreDestroy interceptor or @PreDestroy callback, (2) a producer has a disposal method and (3) a synthetic bean has some destruction logic OR if the bean has a @Dependent dependency that matches the rules (1), (2) or (3).
In your case, there is a producer with no disposer and no @Dependent depencency == no leak ;-). But of course it's always safer to destroy the bean after use.
CC @manovotn
In your case, there is a producer with no disposer and no @Dependent depencency == no leak ;-)
Awesome 🎉
Thanks for the detailed explanation!
I expected a potential memory leak in this extension but my profiler showed a healthy memory so I may be missing something... 😅
So both Weld and ArC try to mitigate this problem in a way that only some references to
@Dependentbeans are stored in theCreationalContext.In ArC, we only add a
@Dependentbean to theCreationalContextif (1) a class bean has@PreDestroyinterceptor or@PreDestroycallback, (2) a producer has a disposal method and (3) a synthetic bean has some destruction logic OR if the bean has a@Dependentdependency that matches the rules (1), (2) or (3).In your case, there is a producer with no disposer and no
@Dependentdepencency == no leak ;-). But of course it's always safer to destroy the bean after use.CC @manovotn
Yea, I can confirm this is the case even for Weld (code here). This is both, a workaround to reduce a chance of memory leak because of bad handling and an optimization as there is no real need to keep the reference if the destruction is a no-op.
That being said, you should handle the bean properly and destroy it when no longer needed (assuming it is intended that those beans are dependent and repeated invocations generate new instances OFC).