[GitHub] [sling-org-apache-sling-rewriter] sagarmiglani commented on a change in pull request #5: SLING-11103 - Improvements in synchronization and additional logging …

[GitBox <gi...@apache.org>]

sagarmiglani commented on a change in pull request #5:
URL: https://github.com/apache/sling-org-apache-sling-rewriter/pull/5#discussion_r796288637



##########
File path: src/main/java/org/apache/sling/rewriter/impl/TransformerFactoryServiceTracker.java
##########
@@ -46,7 +50,7 @@ private boolean isGlobal(final ServiceReference ref) {
     private TransformerFactoryEntry[][] cached = EMPTY_DOUBLE_ENTRY_ARRAY;
 
     /** flag for cache. */
-    private boolean cacheIsValid = true;
+    private volatile boolean cacheIsValid = true;

Review comment:
       Hi Guys,
   I think, even if we use AtomicBoolean(compareAndSet) there is a possibily of error.
   - Let's say a service is removed and cacheIsValid is set to false.
   - Now, there is a call for getting the Global factories which enters the sync block. At this time if a service is added and sets the cache variable to false, the get method at the end will set the cacheIsValid to true and cache will not be updated with the newly added service. So, I believe we should update the cache variable only inside a sync block.
   Please let me know if I am missing something. 




-- 
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-unsubscribe@sling.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org