That was very helpful, thank you. Since requests would be (eventually) garbage collected by the JVM, I feel that a reference/queue-only approach could be sufficient for all types of cleanup. Sure, the servlet listener method could improve the timing of when the cleanup executes, but I imagine it is not necessary (unless something, somewhere in the servlet framework plumbing is caching request objects and reusing them...which I hope not).
I appreciate the insights! //Paul On Wed, Jul 26, 2023 at 10:50 AM Carsten Ziegeler <cziege...@apache.org> wrote: > A while back we had a similar discussion. Now I think this distinction > between real and fake requests could be completely removed, simplifying > the models impl a lot - if we enhance the contract for servlet listeners > being also called by Sling Engine for internal requests (which often use > those fake request objects). > > Regards > Carsten > > On 26.07.2023 17:29, Dirk Rudolph wrote: > > Hey Paul, > > > > afaik the marker attribute it used to distinguish between real and > > synthetic request objects. > > > > The ServletRequestListner implementation of the ModelAdapterFactory is > > only called for requests created by the servlet container. > > > > There are use cases that do not have access to a request object and > > create a synthetic instance to adapt it to a Sling Model (directly or > > indirectly), for example using the SlingHttpServletRequestBuilder [1]. > > In this case requestDestroyed is never called with that request object > > and hence the cleanup mechanism needs to fall back to the one used for > > non-Request adaptables. > > > > Best regards, > > Dirk > > > > [1]: > https://sling.apache.org/apidocs/sling12/org/apache/sling/api/request/builder/SlingHttpServletRequestBuilder.html > > > > On Wed, 19 Jul 2023 at 22:11, Paul Bjorkstrand > > <paul.bjorkstr...@gmail.com> wrote: > >> > >> I'm looking into a potential refactor of the Sling Models Impl to make > the > >> code easier to manage, but also with an eye to enhancing the > functionality > >> in some ways that are hard to do with the way the code is currently > >> structured Examples of such improvements: support other "container > types" > >> in a generic fashion (Optional, WeakReference, and all > Collections/arrays) > >> and implement a java.lang.invoke.MethodHandle based injection system. > One > >> area I have almost no knowledge about is the cleanup side with the > disposal > >> registry, and its related functionality. > >> > >> As I have been reading through the code, I immediately noticed that > there > >> are two paths that are taken, depending on a couple factors, most > notably > >> whether the adaptable is a request or not. At first glance, it seems > that > >> this decision is a necessary difference between request adapted models > or > >> resource adapted models. Digging in deeper, I believe this is not the > case, > >> and it is just a performance/reliability improvement. This makes it not > >> necessary to rely on PhantomReference for request adapted models, since > >> there is a servlet request listener that notifies creation and cleanup > of > >> the requests. This seems to be confirmed after reading [1]. > >> > >> What I don't quite understand is the purpose of REQUEST_MARKER_ATTRIBUTE > >> [2] [3] [4] [5]. What does that do, and why is it needed? Any help > >> understanding this is appreciated! > >> > >> [1]: https://issues.apache.org/jira/browse/SLING-5668 > >> [2]: > >> > https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L106C33-L106C58 > >> [3]: > >> > https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L688 > >> [4]: > >> > https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L774 > >> [5]: > >> > https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L1392 > >> > >> // Paul > > -- > Carsten Ziegeler > Adobe > cziege...@apache.org >
