Memory-"Leak" in Provider / Instance

[Arne Limburg <ar...@openknowledge.de>]

Hi all,

we are migrating an application from a very old version of OWB to the current one and are experiencing huge memory problems.
They are caused by the following pattern, that is widely used in the application:

@ApplicationScoped
public class MyService {

    @Inject
    @Current // custom qualifier
    private Provider<User> currentUserProvider;

    public void doSomething withCurrentUser() {
        User currentUser = currentUserProvider.get();
        …
    }
}

Unfortunately in the Provider (in InstanceImpl) the created objects are stored together with their CreationalContext in a map. I guess, that is required to correctly implement Instance#destroy.
However that leads to the situation, that every user is stored forever in that map and will never be removed, which leads to our memory problems.

I know that the correct solution would be to Instance#destroy the User after usage. And we probably will do that in the migration project.

Although I don’t consider this observed behavior a bug, I wonder if we could deal with that situation in a more intelligent way. I can think of three possible ways:

 1.  When the injection point is of type Provider<T> (and not Instance<T>) we should not store the creational context at all, because there is no way, that it could ever be used.
 2.  The map where the creational contexts are stored could be a WeakHashMap. When the created object is gone there is no way to call Instance#destroy, so we don’t need the creational context.
 3.  We could detect, if a bean has any destruction logic at all, if not, we should not store the creational context, because Instance#destroy is a NoOp anyway

WDYT? If you are fine, I would implement one of the routes. Which one do you prefer?

Cheers,
Arne
OPEN KNOWLEDGE GmbH
Poststraße 1, 26122 Oldenburg
Mobil: +49 151 - 108 22 942
Tel: +49 441 - 4082-154
Fax: +49 441 - 4082-111
arne.limburg@openknowledge.de<ma...@openknowledge.de>
www.openknowledge.de<https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.openknowledge.de%2F&data=04%7C01%7C%7C3004d8758be44c8678c008d93bcc1e23%7C48837bc476f9481d8a76bd7b60b43dec%7C0%7C0%7C637606570139932909%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9vRVYZVZ%2Feqk%2BvFxU5COofNvgs8U0AxtxRqwVEwqXHA%3D&reserved=0>
Registergericht: Amtsgericht Oldenburg, HRB 4670
Geschäftsführer: Lars Röwekamp, Jens Schumann

Treffen Sie uns auf kommenden Konferenzen und Workshops:
Zu unseren Events<https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.openknowledge.de%2Fevent%2F&data=04%7C01%7C%7C3004d8758be44c8678c008d93bcc1e23%7C48837bc476f9481d8a76bd7b60b43dec%7C0%7C0%7C637606570139932909%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=8tjmukdm1NxhXQMkn4VnESiBI216kXvh%2Fjb7%2FFYI0kE%3D&reserved=0>

Re: Memory-"Leak" in Provider / Instance

[Romain Manni-Bucau <rm...@gmail.com>]

Hi Arne,


Le mar. 10 août 2021 à 08:18, Arne Limburg <ar...@openknowledge.de>
a écrit :

> Hi all,
>
> we are migrating an application from a very old version of OWB to the
> current one and are experiencing huge memory problems.
> They are caused by the following pattern, that is widely used in the
> application:
>
> @ApplicationScoped
> public class MyService {
>
>     @Inject
>     @Current // custom qualifier
>     private Provider<User> currentUserProvider;
>
>     public void doSomething withCurrentUser() {
>         User currentUser = currentUserProvider.get();
>         …
>     }
> }
>
> Unfortunately in the Provider (in InstanceImpl) the created objects are
> stored together with their CreationalContext in a map. I guess, that is
> required to correctly implement Instance#destroy.
> However that leads to the situation, that every user is stored forever in
> that map and will never be removed, which leads to our memory problems.
>
> I know that the correct solution would be to Instance#destroy the User
> after usage. And we probably will do that in the migration project.
>
> Although I don’t consider this observed behavior a bug, I wonder if we
> could deal with that situation in a more intelligent way. I can think of
> three possible ways:
>
>  1.  When the injection point is of type Provider<T> (and not Instance<T>)
> we should not store the creational context at all, because there is no way,
> that it could ever be used.
>

I think it is acceptable if the Provider is handled by us - which is the
case of your issue - and not produced by user code (yes it is allowed :D).
However, strictly speaking the cc must be released when the enclosing
instance is destroyed at least.
It is the only way to release the dependent injections so maybe ensure
there is such a test which will prevent functional leaks.


>  2.  The map where the creational contexts are stored could be a
> WeakHashMap. When the created object is gone there is no way to call
> Instance#destroy, so we don’t need the creational context.
>

-1, it generally just creates bugs and will likely lead to cases where
destroy is not functional (we have it in microprofile-config and it hurts a
lot and is quite easy to do if the provider underlying instance is not
stored in the caller which does not mean it must be immediately
destroyed/threading case - depends the codepath), it is also very fragile
in time and depending the JVM will behave differently (depending the
GC+setup to be more accurate)


>  3.  We could detect, if a bean has any destruction logic at all, if not,
> we should not store the creational context, because Instance#destroy is a
> NoOp anyway
>

This one is hard, it requires to check the bytecode I think...in all the
classpath and do codepath analysis, with all the false positives of sonar
and friends I don't think it is a real option (destroy can be in any jar
and indirected quite easily so only trivial case can be detected). So
static analysis will not enable us to know AFAIK.


Wonder if we should have a toggle: "openwebbeans.provider.implementation" -
or whatever name fits you - which overrides "openwebbeans.provider.<fqn
ip>.implementation" to be able to switch between both.
I can see cases where the cleanup is done by some "magic code"
(reflection+AOP for what I have in mind) since providers are instances for
us.
Such a code popped up when CDI 2 appeared.

Still thinking out loud: maybe the factory must be a SPI enabling to
override the impl when needed.


>
> WDYT? If you are fine, I would implement one of the routes. Which one do
> you prefer?
>

option 4 the SPI? I don't see how others can help generally speaking and I
guess a SPI is simple enough to fix your app, wdyt?
Did you try to write an extension which captures the provider injections
and replace them by beans? think it will solve your issue without any OWB
hack and properly since you can handle which injection you manage this way.



>
> Cheers,
> Arne
> OPEN KNOWLEDGE GmbH
> Poststraße 1, 26122 Oldenburg
> Mobil: +49 151 - 108 22 942
> Tel: +49 441 - 4082-154
> Fax: +49 441 - 4082-111
> arne.limburg@openknowledge.de<ma...@openknowledge.de>
> www.openknowledge.de<
> https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.openknowledge.de%2F&data=04%7C01%7C%7C3004d8758be44c8678c008d93bcc1e23%7C48837bc476f9481d8a76bd7b60b43dec%7C0%7C0%7C637606570139932909%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9vRVYZVZ%2Feqk%2BvFxU5COofNvgs8U0AxtxRqwVEwqXHA%3D&reserved=0
> >
> Registergericht: Amtsgericht Oldenburg, HRB 4670
> Geschäftsführer: Lars Röwekamp, Jens Schumann
>
> Treffen Sie uns auf kommenden Konferenzen und Workshops:
> Zu unseren Events<
> https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.openknowledge.de%2Fevent%2F&data=04%7C01%7C%7C3004d8758be44c8678c008d93bcc1e23%7C48837bc476f9481d8a76bd7b60b43dec%7C0%7C0%7C637606570139932909%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=8tjmukdm1NxhXQMkn4VnESiBI216kXvh%2Fjb7%2FFYI0kE%3D&reserved=0
> >
>
>