payara / Payara

Payara Server is an open source middleware platform that supports reliable and secure deployments of Java EE (Jakarta EE) and MicroProfile applications in any environment: on premise, in the cloud or hybrid.
http://www.payara.fish
Other
883 stars 307 forks source link

Payara leaking memory (CDI proxies) when using @Inject @ConfigProperty #3393

Closed lreimer closed 6 years ago

lreimer commented 6 years ago

Description


If using MicroProfile Config API to inject configuration properties via @Inject @ConfigProperty, we found that CDI proxy instances from Weld are leaking and accumulating over time. We found the issue in Payara 4.1.2181, but the problem can also be reproduced using latest 5.183 Payara Micro.

We experimented using @Dependent scope for everything, but that was really not an option, since it will lead to super bad performance due to excessive object creation on every request.

Expected Outcome

No Weld CDI proxy instances are leaking, leading to steady heap and memory usage.

Current Outcome

Over time, more and more Weld CDI proxy instances are leaking, these can not be reclaimed by GC. This leads to increased heap usage with increased GC activity until the JVM is dead and gets OOM killed (by OpenShift).

Steps to reproduce (Only for bug reports)

Please use the supplied sample repository to reproduce this issue.

$ git clone https://github.com/lreimer/mp-config-payara-demo.git
$ cd mp-config-payara-demo
$ ./gradlew ass
$ docker-compose up --build

Connect via JMX on localhost:9090 (no auth, no SSL) using JVisualVM. Turn on memory sampling. Perform GC, and take snapshot.

I am usually using Slapper as load testing tool: slapper --targets leak.targets If not, issue GET requests for the following URLs:

$ curl http://localhost:8080/api/config-property/a.dynamic.string
$ curl http://localhost:8080/api/config-property/a.json.object
$ curl http://localhost:8080/api/config-property/a.long
$ curl http://localhost:8080/api/config-property/a.optional.string
$ curl http://localhost:8080/api/config-property/a.string.array
$ curl http://localhost:8080/api/config-property/hostname

Now, perform GC in VisualVM again, and take another snapshot. Compare the snapshots, now you should see an increase of the following instances.

org.jboss.weld.contexts.CreationalContextImpl || +94.848 B | +1.976
org.jboss.weld.contexts.SerializableContextualInstanceImpl || +42.912 B | +1.788
org.jboss.weld.contexts.SerializableContextualFactory$SerializableContextualHolder || +71.520 B | +1.788
org.jboss.weld.contexts.SerializableContextualFactory$DefaultSerializableBean || +28.544 B | +1.784

I have also attached a screenshot showing the results after letting the load test run for a while.

Samples

The reproducer can be found in this repository: https://github.com/lreimer/mp-config-payara-demo

Context (Optional)

We found the issue during load tests of a Microservice running on OpenShift. After 2 hours first the performance degraded significantly due to excessive GC, and finally the pod was killed due to OOM.

Environment

lreimer commented 6 years ago
screenshot 2018-11-07 22 09 52
lreimer commented 6 years ago

Small update: I did some more experiments. It seems the leak is caused by injecting a Provider for a configuration property:

    @Inject
    @ConfigProperty(name = "a.dynamic.string", defaultValue = "A default value.")
    private Provider<String> aDynamicString;
smillidge commented 6 years ago

I assume the bean being injected into is RequestScoped?

lreimer commented 6 years ago

No. The ConfigurationBean in the reproducer is @ApplicationScoped.

smillidge commented 6 years ago

Yes I was just going to write that having just thought about it. The beans being created by the Provider will be Dependent scoped so will be dependent on the lifecycle of the requestor in this case an ApplicationScoped bean so will hang around until the application is undeployed.

I assume the application is calling get on the Provider multiple times to generate many beans.

smillidge commented 6 years ago

If that is the case the developer should inject a request scoped bean and use that to retrieve the dynamic string on each request?

lreimer commented 6 years ago

Unfortunately, I can not confirm this. Changing the beans (JAX-RS resource bean as well as configuration bean) to @RequestScoped does not make a difference. The leak is still there.

As I have written previously, only changing really everything to @Dependent does not leak any proxies. Well, expected since @Dependent is a no-proxy scope. But the performance is super super bad. So this is not an option.

Also, if the beans and instances created by the provider are Dependent scoped, why is it leaking CDI org.jboss.weld.contexts.CreationalContextImpl instances then? Dependent is a no-proxy scope.

smillidge commented 6 years ago

I think this is due to calling the Provider get many times. Provider isn't a CDI api. I believe replacing the Provider.get with injection of a RequestScopedBean which just uses simple @Inject to inject the string value would solve the memory leak.

My feeling is that you should be using destroy with get which is somewhat tricky as the value is a String. You could potentially use Instance<> instead of Provider as this has the destroy method.

Note I haven't tested this.

lreimer commented 6 years ago

And you are right. I just tested and measured again, with different scope combinations. Making the bean where the config properties are injected @RequestScoped indeed solves it.

Nevertheless, this CDI behaviour somewhat leaves a bad taste, that I can produce a CDI proxy leak with the combination of an @ApplicationScoped bean with

@Inject
    @ConfigProperty(name = "a.dynamic.string", defaultValue = "A default value.")
    private Provider<String> aDynamicString;
smillidge commented 6 years ago

It is possible to create CDI proxy leaks when you are programmatically creating beans like using the Provider api, using Instance or using the BeanManager directly.

You could try;

@Inject
@ConfigProperty(name = "a.dynamic.string", defaultValue = "A default value.")
private Instance<String> aDynamicString;

and call Instance.destroy in the processing method as an alternative to the RequestScopedBean wrapper.

lreimer commented 6 years ago

Tried it. Using Instance does does not work and results in a deployment error.

avax.enterprise.inject.spi.DeploymentException: Deployment Failure for ConfigProperty a.dynamic.string in class cloud.nativ.javaee.ConfigPropertyBean Reason Unable to convert value to type javax.enterprise.inject.Instance
config-service_1  |     at fish.payara.microprofile.config.cdi.CDIExtension.validateInjectionPoint(CDIExtension.java:92)
...
Caused by: java.lang.IllegalArgumentException: Unable to convert value to type javax.enterprise.inject.Instance
config-service_1  |     at fish.payara.nucleus.microprofile.config.spi.PayaraConfig.getConverter(PayaraConfig.java:241)
config-service_1  |     at fish.payara.nucleus.microprofile.config.spi.PayaraConfig.convertString(PayaraConfig.java:264)
config-service_1  |     at fish.payara.nucleus.microprofile.config.spi.PayaraConfig.getValue(PayaraConfig.java:87)
config-service_1  |     at fish.payara.microprofile.config.cdi.ConfigPropertyProducer.getGenericProperty(ConfigPropertyProducer.java:105)
config-service_1  |     at fish.payara.microprofile.config.cdi.CDIExtension.validateInjectionPoint(CDIExtension.java:81)
smillidge commented 6 years ago

I'll close this as it is an application error.

lreimer commented 6 years ago

I would not exactly call this an application error, but OK with me. I still find it more than unexpected that using plain String config properties via Provider<String> leads to a CDI proxy leak. Thanks anyway for the discussion and the solution using @RequestScoped.

flobuc commented 1 year ago

I have the same memory leak problem with using Provider<String> in @ApplicationScoped beans. How is the mentioned workaround working? Injecting a Provider<String> in a @RequestedScoped bean and this bean in a @ApplicationScoped bean?

smillidge commented 1 year ago

I assume you are using a Provider rather than direct String injection in the @ApplicationScoped bean because you want the value to change between requests and not just be read once?

Therefore if you use a @RequestScopedBean and inject it into the @ApplicationScopedBean CDI will inject a proxy object. That proxy object will ensure that everytime you read the value of the Proxy it will bind to the appropriate instance of the bean and you will get the updated value. CDI will then correctly clean up any proxies.

Using the Provider you are implicitly requesting the Provider to inject you a new value on every access but there is no way to clean these up as they are lifetime dependent on the scope of the bean requesting them i.e. the lifetime of the Application.

flobuc commented 1 year ago

Thanks, for your explanation. I got it. Nevertheless, i agree with @lreimer. This BUG leads to a lot of trouble in our case and i think it's sad that this issue was closed as an application error. If i have time i will check, how other microprofile Implementations handle that case.