I'm adding a feature to some longstanding, complex integration code in AEM 6.5 and discovering existing problems with invalid resource resolvers. Adding the feature is making them worse. This is some of the oldest code in our system, and has gone through several rounds of refactoring and other updates.
I'm unclear about the best way to manage the resource resolver and associated objects, especially as control goes from a service to Sling models that use a resolver or session to traverse or search the JCR. I'm hoping someone can review my code and give me some advice (@Jörg_Hoh , possible to get your thoughts?) Here's the general overview, followed by VERY stripped-down versions of the classes involved:
There are two types of models: a mostly POJO "index" model and familiar Sling Model "entities" which read data from the JCR. The index models instantiate various entity models, pull various model properties into a map, and that map gets serialized to JSON and sent to the external DB. The index models don't declare a resolver -- I'm not clear on what they're using when they do path traversal. The entity models declare an injected @SlingObject ResourceResolver.
I'm getting "Resource resolver is already closed" from the scheduled task when calling getPath() or listChildren() on a resource.
I'm getting "Resource resolver is already closed" and "This session has been closed" exceptions from the entity models when they try to do path traversal via getParent(), or when the upload service is trying to execute a query.
Any help appreciated!
Solved! Go to Solution.
Topics help categorize Community content and increase your ability to discover relevant content.
Views
Replies
Total Likes
The problem in in the MyScheduledTask class is that it is an OSGI component, and therefor a singleton. That means, that the resourceResolver field is shared amongst all invocations of the run() method. As this is a scheduled job, there is a chance that it is invoked (again) even if the previous invocation has not yet completed.
And that means, that at one point you close the ResourceResolver (and reopen it again), but then every reference to that now closed ResourceResolver is invalid, and you get the exception you are facing.
I would implement it like this:
public void run() {
try (ResourceResolver resourceResolver = getResourceResolver;){
Resource msgFolder = resourceResolver.getResource("/path/to/message/nodes");
[...]
} catch (Exception e) {
logger.error(e);
}
}
In this case the ResourceResolver is private to every invocation, and you won't get that problem. Of course you need to pass the ResourceResolver then to every method.
In the marketEntity the problem is less obvious. I think that you use the same approach as above, but then you pass a Resource into a such a SlingModel, and that SlingModel lives longer than this ResourceResolver. In this example I would only store a simple String as the path property of the Model, not the resource itself (it does not bring any value).
@valcohen avoid global variables for resourceresolver, try to declare it local to method scope and return the object when method invoked.. avoid using this for resourceresolver..
Do you have any specific examples?I don't think I've seen this approach, can you show me how you'd change one of these classes to the style you recommend, or point me to some examples? Do I still inject the resolverFactory, but just call it in every method that needs it, and then close the resolver the method retrieved at the end of each method? How do I initialize the resolver in the Sling models in the absence of a SlingHttpRequest?
The problem in in the MyScheduledTask class is that it is an OSGI component, and therefor a singleton. That means, that the resourceResolver field is shared amongst all invocations of the run() method. As this is a scheduled job, there is a chance that it is invoked (again) even if the previous invocation has not yet completed.
And that means, that at one point you close the ResourceResolver (and reopen it again), but then every reference to that now closed ResourceResolver is invalid, and you get the exception you are facing.
I would implement it like this:
public void run() {
try (ResourceResolver resourceResolver = getResourceResolver;){
Resource msgFolder = resourceResolver.getResource("/path/to/message/nodes");
[...]
} catch (Exception e) {
logger.error(e);
}
}
In this case the ResourceResolver is private to every invocation, and you won't get that problem. Of course you need to pass the ResourceResolver then to every method.
In the marketEntity the problem is less obvious. I think that you use the same approach as above, but then you pass a Resource into a such a SlingModel, and that SlingModel lives longer than this ResourceResolver. In this example I would only store a simple String as the path property of the Model, not the resource itself (it does not bring any value).
Hi Jörg, thanks for the reply! I'll try the try-with-resources approach -- it's a bit of a pain to pass the resolver in to all the methods, but I see your point about the singleton (the job runs once a minute, and if there's a heavy load of inbound messages the processing can definitely take longer than that). I assume I can just pass the resolver, and if I need to, adapt it to a QueryBuilder, PageManager, Session, etc. within the methods that need those objects, right? Or do I need to adapt them all when the resolver is created, and pass them separately?
I'm not clear on the comments about the Sling models, and that part is even harder for me to understand. The models need a resolver because some of their methods need to do traversal or queries. I may be able to precompute a path, but lots of other processing is encapsulated in the models. I currently inject the resolver into the Sling model base class, but don't really understand what resolver is being used there (what credentials does it carry?). Should I inject a ResolverFactory instead, and get a resolver inside the model's various methods? How can I manage its lifecycle to make sure it's closed when the model? Should I get a resolver in PostConstruct, and create a PreDestroy method to close it?
Thanks again for dropping in, I really appreciate the explanations and examples!
Sling Models are normally created by adapting either a resource or a request to that SlingModel. So when it comes to injecting a ResourceResolver into a Sling Model class, there it's quite natural which ResourceResolver will be taken:
In the 1st case you need to make sure that the ResourceResolver is open as long as the model can be used (in the 2nd case this is implicitly the case).
does that help?
Jörg
(Would be a nice topic for a blog post)
Yes, that helps a lot, I'll give these techniques a try. My module is long overdue for a rewrite, I guess now is as good a time as any.
I'd love to read a blog about how you'd implement a design like this -- most of what I've seen is around adapting SlingHttpRequests and sending back the response, rather than adapting resources in background jobs. Thanks again!
Views
Likes
Replies
Views
Likes
Replies
Views
Likes
Replies