sagarmiglani commented on a change in pull request #34:
URL:
https://github.com/apache/sling-org-apache-sling-models-impl/pull/34#discussion_r803384095
##########
File path:
src/main/java/org/apache/sling/models/impl/injectors/OSGiServiceInjector.java
##########
@@ -204,7 +204,11 @@ public Callback(ServiceReference<?>[] refs, BundleContext
context) {
public void onDisposed() {
if (refs != null) {
for (ServiceReference<?> ref : refs) {
- context.ungetService(ref);
+ try {
+ context.ungetService(ref);
+ } catch (IllegalStateException | IllegalArgumentException
| NullPointerException exception) {
Review comment:
Hi Kwin, Thanks for the comment. I agree that NPE, IAE or any other
runtime exception should not be caught unless we can recover safely from it.
But point was here to let the dispose the all references present in the queue
(0) at once (if present). I will remove the NPE and IAE from catch block. I
have tried to explain the scenario in JIRA (1).
Whenever we use a model (let's a M in Bundle B) with OSGi service injection
and use the model in a page.. OSGiServices are injected into the model and for
disposing these injected references ModelAdapterFactory keeps PhantomReferences
with reference queue (see ModelAdapterFactory for more details (2)). This queue
is checked every 30 seconds (default), to disposes these references. And if
Bundle B is refreshed (restarted) before the references were disposed, while
disposing these references (context.ungetService(ref)) we get ISE: Invalid
Bundle Context. which stops the current job trigger and the next reference is
picked in the next job trigger.
Please let me If I am missing something.
0:
https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L210
1: https://issues.apache.org/jira/browse/SLING-11132
2:
https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
##########
File path:
src/main/java/org/apache/sling/models/impl/injectors/OSGiServiceInjector.java
##########
@@ -204,7 +204,11 @@ public Callback(ServiceReference<?>[] refs, BundleContext
context) {
public void onDisposed() {
if (refs != null) {
for (ServiceReference<?> ref : refs) {
- context.ungetService(ref);
+ try {
+ context.ungetService(ref);
+ } catch (IllegalStateException | IllegalArgumentException
| NullPointerException exception) {
Review comment:
Hi Kwin, Thanks for the comment. I agree that NPE, IAE or any other
runtime exception should not be caught unless we can recover safely from it.
But point was here to let the dispose the all references present in the queue
(0) at once (if present). I will remove the NPE and IAE from catch block. I
have tried to explain the scenario in JIRA (1).
Whenever we use a model (let's a M in Bundle B) with OSGi service injection
and use the model in a page.. OSGiServices are injected into the model and for
disposing these injected references ModelAdapterFactory keeps PhantomReferences
with reference queue (see ModelAdapterFactory for more details (2)). This queue
is checked every 30 seconds (default), to disposes these references. And if
Bundle B is refreshed (restarted) before the references were disposed, while
disposing these references (context.ungetService(ref)) we get ISE: Invalid
Bundle Context. which stops the current job trigger and the next reference is
picked in the next job trigger.
Please let me know If I am missing something.
0:
https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L210
1: https://issues.apache.org/jira/browse/SLING-11132
2:
https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
