You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Jörg Hoh <jh...@googlemail.com.INVALID> on 2023/05/26 14:41:16 UTC
Synthethic requests and Sling Models
Hi,
In AEM we have for quite some time "fake" implementations of
SlingHttpServletRequest and SlingHttpServletResponse, which are used as
parameters for the SlingProcessor to render content.
We came across issues when SlingModels are involved in the rendering
process. We found that this approach can create memory leaks when OSGI
services are injected into sling models, and these are created during the
rendering process of such a "fake request". The problem is that the
unregistering of these OSGI references is triggered by a
ServletRequestEvent which the ModelAdapterFactory receives as it registers
as a ServletRequestListener. But these fake classes do not implement these
methods at all.
Checking the sling equivalents in [1] I don't see them implementing these
functionality as well, thus they show the same problem. Would there be some
interest to have this functionality implemented in Sling? (I am not an
expert in the servlet spec, so don't expect anything soon from my side...)
Jörg
[1]
https://github.com/apache/sling-org-apache-sling-api/tree/master/src/main/java/org/apache/sling/api/request/builder
--
Cheers,
Jörg Hoh,
https://cqdump.joerghoh.de
Twitter: @joerghoh
Re: Synthethic requests and Sling Models
Posted by Carsten Ziegeler <cz...@apache.org>.
But getting back to the original topic that started this thread,
https://issues.apache.org/jira/browse/SLING-7517 should address that case.
@Jörg can you maybe check whether that is actually working?
Regards
Carsten
On 30.05.2023 09:32, Carsten Ziegeler wrote:
>
>
> On 30.05.2023 06:23, Carsten Ziegeler wrote:
>> We could extend the contract for SlingRequestListener - which is
>> called for the main request already from Sling Engine and let it call
>> those listeners for internal requests as well (with new types).
>> If we want to be on the save side, we could require a special
>> registration property to opt-in for listeners.
>>
>
> Looking at the Sling models implementation, with such a consistent
> listener support the whole disposing mechanism in Sling models could be
> simplified (e.g. by using a Stack of callbacks - which avoids the
> current thread local and reference handling completely).
>
> Regards
> Carsten
>
>> Regards
>> Carsten
>>
>> On 27.05.2023 11:40, Carsten Ziegeler wrote:
>>> Hi,
>>>
>>> I fear this will not work :) the fake/internal request might be
>>> within the context of a real request or outside of it. However, if it
>>> is within the context of a real request, it might share the request
>>> attributes. In that case, for example the servlet request listener
>>> from the auth core bundle will close the resource resolver. Sling
>>> Models might also not be prepared for it. Other listeners might do
>>> the wrong thing, too.
>>>
>>> In other words, I don't think we can find a way that magically does
>>> the right thing.
>>>
>>> The problem that seems to need fixing is clean up of resources when
>>> Sling Models are used. So maybe it is better/safer to concentrate on
>>> just that.
>>>
>>> Regards
>>> Carsten
>>>
>>>
>>> On 26.05.2023 18:33, Konrad Windszus wrote:
>>>> I added a draft in
>>>> https://github.com/apache/sling-org-apache-sling-engine/pull/35/files#.
>>>> Feel free to have a look and take over if you deem that useful.
>>>> Haven’t tested it yet…
>>>>
>>>> Konrad
>>>>
>>>>> On 26. May 2023, at 17:32, Konrad Windszus <kw...@apache.org> wrote:
>>>>>
>>>>> I think this can be implemented with the
>>>>> HttpServiceRuntime.getRuntimeDRO[1] which should return all
>>>>> registered event listeners [2] through its ServletContextDTO (also
>>>>> the ServletRequestListener registered by ModelAdapterFactory).
>>>>> The question is though when to call the according events, as the
>>>>> synthetic SlingHttpServletRequest has no explicit close method.
>>>>> This probably needs to be done by the SlingProcessor which needs to
>>>>> distinguish between requests created by the servlet container and
>>>>> these virtual requests.
>>>>> Don’t know how to distinguish those properly to check that, maybe
>>>>> by evaluating the requests getServletContext() return value to
>>>>> check if this is a fake context or a real one…
>>>>>
>>>>> Konrad
>>>>>
>>>>> [1]
>>>>> https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/HttpServiceRuntime.html#getRuntimeDTO()
>>>>> [2]
>>>>> https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/dto/ListenerDTO.html
>>>>>
>>>>>> On 26. May 2023, at 16:41, Jörg Hoh
>>>>>> <jh...@googlemail.com.INVALID> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> In AEM we have for quite some time "fake" implementations of
>>>>>> SlingHttpServletRequest and SlingHttpServletResponse, which are
>>>>>> used as
>>>>>> parameters for the SlingProcessor to render content.
>>>>>>
>>>>>> We came across issues when SlingModels are involved in the rendering
>>>>>> process. We found that this approach can create memory leaks when
>>>>>> OSGI
>>>>>> services are injected into sling models, and these are created
>>>>>> during the
>>>>>> rendering process of such a "fake request". The problem is that the
>>>>>> unregistering of these OSGI references is triggered by a
>>>>>> ServletRequestEvent which the ModelAdapterFactory receives as it
>>>>>> registers
>>>>>> as a ServletRequestListener. But these fake classes do not
>>>>>> implement these
>>>>>> methods at all.
>>>>>>
>>>>>> Checking the sling equivalents in [1] I don't see them
>>>>>> implementing these
>>>>>> functionality as well, thus they show the same problem. Would
>>>>>> there be some
>>>>>> interest to have this functionality implemented in Sling? (I am
>>>>>> not an
>>>>>> expert in the servlet spec, so don't expect anything soon from my
>>>>>> side...)
>>>>>>
>>>>>>
>>>>>> Jörg
>>>>>>
>>>>>>
>>>>>> [1]
>>>>>> https://github.com/apache/sling-org-apache-sling-api/tree/master/src/main/java/org/apache/sling/api/request/builder
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Cheers,
>>>>>> Jörg Hoh,
>>>>>>
>>>>>> https://cqdump.joerghoh.de
>>>>>> Twitter: @joerghoh
>>>>>
>>>>
>>>>
>>>
>>
>
--
Carsten Ziegeler
Adobe
cziegeler@apache.org
Re: Synthethic requests and Sling Models
Posted by Carsten Ziegeler <cz...@apache.org>.
On 30.05.2023 06:23, Carsten Ziegeler wrote:
> We could extend the contract for SlingRequestListener - which is called
> for the main request already from Sling Engine and let it call those
> listeners for internal requests as well (with new types).
> If we want to be on the save side, we could require a special
> registration property to opt-in for listeners.
>
Looking at the Sling models implementation, with such a consistent
listener support the whole disposing mechanism in Sling models could be
simplified (e.g. by using a Stack of callbacks - which avoids the
current thread local and reference handling completely).
Regards
Carsten
> Regards
> Carsten
>
> On 27.05.2023 11:40, Carsten Ziegeler wrote:
>> Hi,
>>
>> I fear this will not work :) the fake/internal request might be within
>> the context of a real request or outside of it. However, if it is
>> within the context of a real request, it might share the request
>> attributes. In that case, for example the servlet request listener
>> from the auth core bundle will close the resource resolver. Sling
>> Models might also not be prepared for it. Other listeners might do the
>> wrong thing, too.
>>
>> In other words, I don't think we can find a way that magically does
>> the right thing.
>>
>> The problem that seems to need fixing is clean up of resources when
>> Sling Models are used. So maybe it is better/safer to concentrate on
>> just that.
>>
>> Regards
>> Carsten
>>
>>
>> On 26.05.2023 18:33, Konrad Windszus wrote:
>>> I added a draft in
>>> https://github.com/apache/sling-org-apache-sling-engine/pull/35/files#.
>>> Feel free to have a look and take over if you deem that useful.
>>> Haven’t tested it yet…
>>>
>>> Konrad
>>>
>>>> On 26. May 2023, at 17:32, Konrad Windszus <kw...@apache.org> wrote:
>>>>
>>>> I think this can be implemented with the
>>>> HttpServiceRuntime.getRuntimeDRO[1] which should return all
>>>> registered event listeners [2] through its ServletContextDTO (also
>>>> the ServletRequestListener registered by ModelAdapterFactory).
>>>> The question is though when to call the according events, as the
>>>> synthetic SlingHttpServletRequest has no explicit close method.
>>>> This probably needs to be done by the SlingProcessor which needs to
>>>> distinguish between requests created by the servlet container and
>>>> these virtual requests.
>>>> Don’t know how to distinguish those properly to check that, maybe by
>>>> evaluating the requests getServletContext() return value to check if
>>>> this is a fake context or a real one…
>>>>
>>>> Konrad
>>>>
>>>> [1]
>>>> https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/HttpServiceRuntime.html#getRuntimeDTO()
>>>> [2]
>>>> https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/dto/ListenerDTO.html
>>>>
>>>>> On 26. May 2023, at 16:41, Jörg Hoh
>>>>> <jh...@googlemail.com.INVALID> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> In AEM we have for quite some time "fake" implementations of
>>>>> SlingHttpServletRequest and SlingHttpServletResponse, which are
>>>>> used as
>>>>> parameters for the SlingProcessor to render content.
>>>>>
>>>>> We came across issues when SlingModels are involved in the rendering
>>>>> process. We found that this approach can create memory leaks when OSGI
>>>>> services are injected into sling models, and these are created
>>>>> during the
>>>>> rendering process of such a "fake request". The problem is that the
>>>>> unregistering of these OSGI references is triggered by a
>>>>> ServletRequestEvent which the ModelAdapterFactory receives as it
>>>>> registers
>>>>> as a ServletRequestListener. But these fake classes do not
>>>>> implement these
>>>>> methods at all.
>>>>>
>>>>> Checking the sling equivalents in [1] I don't see them implementing
>>>>> these
>>>>> functionality as well, thus they show the same problem. Would there
>>>>> be some
>>>>> interest to have this functionality implemented in Sling? (I am not an
>>>>> expert in the servlet spec, so don't expect anything soon from my
>>>>> side...)
>>>>>
>>>>>
>>>>> Jörg
>>>>>
>>>>>
>>>>> [1]
>>>>> https://github.com/apache/sling-org-apache-sling-api/tree/master/src/main/java/org/apache/sling/api/request/builder
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Cheers,
>>>>> Jörg Hoh,
>>>>>
>>>>> https://cqdump.joerghoh.de
>>>>> Twitter: @joerghoh
>>>>
>>>
>>>
>>
>
--
Carsten Ziegeler
Adobe
cziegeler@apache.org
Re: Synthethic requests and Sling Models
Posted by Carsten Ziegeler <cz...@apache.org>.
We could extend the contract for SlingRequestListener - which is called
for the main request already from Sling Engine and let it call those
listeners for internal requests as well (with new types).
If we want to be on the save side, we could require a special
registration property to opt-in for listeners.
Regards
Carsten
On 27.05.2023 11:40, Carsten Ziegeler wrote:
> Hi,
>
> I fear this will not work :) the fake/internal request might be within
> the context of a real request or outside of it. However, if it is within
> the context of a real request, it might share the request attributes. In
> that case, for example the servlet request listener from the auth core
> bundle will close the resource resolver. Sling Models might also not be
> prepared for it. Other listeners might do the wrong thing, too.
>
> In other words, I don't think we can find a way that magically does the
> right thing.
>
> The problem that seems to need fixing is clean up of resources when
> Sling Models are used. So maybe it is better/safer to concentrate on
> just that.
>
> Regards
> Carsten
>
>
> On 26.05.2023 18:33, Konrad Windszus wrote:
>> I added a draft in
>> https://github.com/apache/sling-org-apache-sling-engine/pull/35/files#.
>> Feel free to have a look and take over if you deem that useful.
>> Haven’t tested it yet…
>>
>> Konrad
>>
>>> On 26. May 2023, at 17:32, Konrad Windszus <kw...@apache.org> wrote:
>>>
>>> I think this can be implemented with the
>>> HttpServiceRuntime.getRuntimeDRO[1] which should return all
>>> registered event listeners [2] through its ServletContextDTO (also
>>> the ServletRequestListener registered by ModelAdapterFactory).
>>> The question is though when to call the according events, as the
>>> synthetic SlingHttpServletRequest has no explicit close method.
>>> This probably needs to be done by the SlingProcessor which needs to
>>> distinguish between requests created by the servlet container and
>>> these virtual requests.
>>> Don’t know how to distinguish those properly to check that, maybe by
>>> evaluating the requests getServletContext() return value to check if
>>> this is a fake context or a real one…
>>>
>>> Konrad
>>>
>>> [1]
>>> https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/HttpServiceRuntime.html#getRuntimeDTO()
>>> [2]
>>> https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/dto/ListenerDTO.html
>>>
>>>> On 26. May 2023, at 16:41, Jörg Hoh <jh...@googlemail.com.INVALID>
>>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> In AEM we have for quite some time "fake" implementations of
>>>> SlingHttpServletRequest and SlingHttpServletResponse, which are used as
>>>> parameters for the SlingProcessor to render content.
>>>>
>>>> We came across issues when SlingModels are involved in the rendering
>>>> process. We found that this approach can create memory leaks when OSGI
>>>> services are injected into sling models, and these are created
>>>> during the
>>>> rendering process of such a "fake request". The problem is that the
>>>> unregistering of these OSGI references is triggered by a
>>>> ServletRequestEvent which the ModelAdapterFactory receives as it
>>>> registers
>>>> as a ServletRequestListener. But these fake classes do not implement
>>>> these
>>>> methods at all.
>>>>
>>>> Checking the sling equivalents in [1] I don't see them implementing
>>>> these
>>>> functionality as well, thus they show the same problem. Would there
>>>> be some
>>>> interest to have this functionality implemented in Sling? (I am not an
>>>> expert in the servlet spec, so don't expect anything soon from my
>>>> side...)
>>>>
>>>>
>>>> Jörg
>>>>
>>>>
>>>> [1]
>>>> https://github.com/apache/sling-org-apache-sling-api/tree/master/src/main/java/org/apache/sling/api/request/builder
>>>>
>>>>
>>>>
>>>> --
>>>> Cheers,
>>>> Jörg Hoh,
>>>>
>>>> https://cqdump.joerghoh.de
>>>> Twitter: @joerghoh
>>>
>>
>>
>
--
Carsten Ziegeler
Adobe
cziegeler@apache.org
Re: Synthethic requests and Sling Models
Posted by Carsten Ziegeler <cz...@apache.org>.
Hi,
I fear this will not work :) the fake/internal request might be within
the context of a real request or outside of it. However, if it is within
the context of a real request, it might share the request attributes. In
that case, for example the servlet request listener from the auth core
bundle will close the resource resolver. Sling Models might also not be
prepared for it. Other listeners might do the wrong thing, too.
In other words, I don't think we can find a way that magically does the
right thing.
The problem that seems to need fixing is clean up of resources when
Sling Models are used. So maybe it is better/safer to concentrate on
just that.
Regards
Carsten
On 26.05.2023 18:33, Konrad Windszus wrote:
> I added a draft in https://github.com/apache/sling-org-apache-sling-engine/pull/35/files#.
> Feel free to have a look and take over if you deem that useful.
> Haven’t tested it yet…
>
> Konrad
>
>> On 26. May 2023, at 17:32, Konrad Windszus <kw...@apache.org> wrote:
>>
>> I think this can be implemented with the HttpServiceRuntime.getRuntimeDRO[1] which should return all registered event listeners [2] through its ServletContextDTO (also the ServletRequestListener registered by ModelAdapterFactory).
>> The question is though when to call the according events, as the synthetic SlingHttpServletRequest has no explicit close method.
>> This probably needs to be done by the SlingProcessor which needs to distinguish between requests created by the servlet container and these virtual requests.
>> Don’t know how to distinguish those properly to check that, maybe by evaluating the requests getServletContext() return value to check if this is a fake context or a real one…
>>
>> Konrad
>>
>> [1] https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/HttpServiceRuntime.html#getRuntimeDTO()
>> [2] https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/dto/ListenerDTO.html
>>
>>> On 26. May 2023, at 16:41, Jörg Hoh <jh...@googlemail.com.INVALID> wrote:
>>>
>>> Hi,
>>>
>>> In AEM we have for quite some time "fake" implementations of
>>> SlingHttpServletRequest and SlingHttpServletResponse, which are used as
>>> parameters for the SlingProcessor to render content.
>>>
>>> We came across issues when SlingModels are involved in the rendering
>>> process. We found that this approach can create memory leaks when OSGI
>>> services are injected into sling models, and these are created during the
>>> rendering process of such a "fake request". The problem is that the
>>> unregistering of these OSGI references is triggered by a
>>> ServletRequestEvent which the ModelAdapterFactory receives as it registers
>>> as a ServletRequestListener. But these fake classes do not implement these
>>> methods at all.
>>>
>>> Checking the sling equivalents in [1] I don't see them implementing these
>>> functionality as well, thus they show the same problem. Would there be some
>>> interest to have this functionality implemented in Sling? (I am not an
>>> expert in the servlet spec, so don't expect anything soon from my side...)
>>>
>>>
>>> Jörg
>>>
>>>
>>> [1]
>>> https://github.com/apache/sling-org-apache-sling-api/tree/master/src/main/java/org/apache/sling/api/request/builder
>>>
>>>
>>>
>>> --
>>> Cheers,
>>> Jörg Hoh,
>>>
>>> https://cqdump.joerghoh.de
>>> Twitter: @joerghoh
>>
>
>
--
Carsten Ziegeler
Adobe
cziegeler@apache.org
Re: Synthethic requests and Sling Models
Posted by Konrad Windszus <kw...@apache.org>.
I added a draft in https://github.com/apache/sling-org-apache-sling-engine/pull/35/files#.
Feel free to have a look and take over if you deem that useful.
Haven’t tested it yet…
Konrad
> On 26. May 2023, at 17:32, Konrad Windszus <kw...@apache.org> wrote:
>
> I think this can be implemented with the HttpServiceRuntime.getRuntimeDRO[1] which should return all registered event listeners [2] through its ServletContextDTO (also the ServletRequestListener registered by ModelAdapterFactory).
> The question is though when to call the according events, as the synthetic SlingHttpServletRequest has no explicit close method.
> This probably needs to be done by the SlingProcessor which needs to distinguish between requests created by the servlet container and these virtual requests.
> Don’t know how to distinguish those properly to check that, maybe by evaluating the requests getServletContext() return value to check if this is a fake context or a real one…
>
> Konrad
>
> [1] https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/HttpServiceRuntime.html#getRuntimeDTO()
> [2] https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/dto/ListenerDTO.html
>
>> On 26. May 2023, at 16:41, Jörg Hoh <jh...@googlemail.com.INVALID> wrote:
>>
>> Hi,
>>
>> In AEM we have for quite some time "fake" implementations of
>> SlingHttpServletRequest and SlingHttpServletResponse, which are used as
>> parameters for the SlingProcessor to render content.
>>
>> We came across issues when SlingModels are involved in the rendering
>> process. We found that this approach can create memory leaks when OSGI
>> services are injected into sling models, and these are created during the
>> rendering process of such a "fake request". The problem is that the
>> unregistering of these OSGI references is triggered by a
>> ServletRequestEvent which the ModelAdapterFactory receives as it registers
>> as a ServletRequestListener. But these fake classes do not implement these
>> methods at all.
>>
>> Checking the sling equivalents in [1] I don't see them implementing these
>> functionality as well, thus they show the same problem. Would there be some
>> interest to have this functionality implemented in Sling? (I am not an
>> expert in the servlet spec, so don't expect anything soon from my side...)
>>
>>
>> Jörg
>>
>>
>> [1]
>> https://github.com/apache/sling-org-apache-sling-api/tree/master/src/main/java/org/apache/sling/api/request/builder
>>
>>
>>
>> --
>> Cheers,
>> Jörg Hoh,
>>
>> https://cqdump.joerghoh.de
>> Twitter: @joerghoh
>
Re: Synthethic requests and Sling Models
Posted by Konrad Windszus <kw...@apache.org>.
I think this can be implemented with the HttpServiceRuntime.getRuntimeDRO[1] which should return all registered event listeners [2] through its ServletContextDTO (also the ServletRequestListener registered by ModelAdapterFactory).
The question is though when to call the according events, as the synthetic SlingHttpServletRequest has no explicit close method.
This probably needs to be done by the SlingProcessor which needs to distinguish between requests created by the servlet container and these virtual requests.
Don’t know how to distinguish those properly to check that, maybe by evaluating the requests getServletContext() return value to check if this is a fake context or a real one…
Konrad
[1] https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/HttpServiceRuntime.html#getRuntimeDTO()
[2] https://docs.osgi.org/javadoc/r6/cmpn/org/osgi/service/http/runtime/dto/ListenerDTO.html
> On 26. May 2023, at 16:41, Jörg Hoh <jh...@googlemail.com.INVALID> wrote:
>
> Hi,
>
> In AEM we have for quite some time "fake" implementations of
> SlingHttpServletRequest and SlingHttpServletResponse, which are used as
> parameters for the SlingProcessor to render content.
>
> We came across issues when SlingModels are involved in the rendering
> process. We found that this approach can create memory leaks when OSGI
> services are injected into sling models, and these are created during the
> rendering process of such a "fake request". The problem is that the
> unregistering of these OSGI references is triggered by a
> ServletRequestEvent which the ModelAdapterFactory receives as it registers
> as a ServletRequestListener. But these fake classes do not implement these
> methods at all.
>
> Checking the sling equivalents in [1] I don't see them implementing these
> functionality as well, thus they show the same problem. Would there be some
> interest to have this functionality implemented in Sling? (I am not an
> expert in the servlet spec, so don't expect anything soon from my side...)
>
>
> Jörg
>
>
> [1]
> https://github.com/apache/sling-org-apache-sling-api/tree/master/src/main/java/org/apache/sling/api/request/builder
>
>
>
> --
> Cheers,
> Jörg Hoh,
>
> https://cqdump.joerghoh.de
> Twitter: @joerghoh