joerghoh commented on code in PR #5:
URL:
https://github.com/apache/sling-org-apache-sling-commons-threads/pull/5#discussion_r1285052784
##########
src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java:
##########
@@ -68,8 +69,14 @@ protected void afterExecute(Runnable r, Throwable t) {
if (cleaner != null) {
cleaner.cleanup();
+ this.cleanupCounter++;
Review Comment:
This is called unconditionally, that means it will increment the counter
even if not cleanup was required. I would suggest (SLING-11149 is totally clear
in this case) to just count the number of occurrences when a cleanup was
actually doing work and releasing ThreadLocals. Then this counter is a useful
metric, because any value != 0 will hint to a problem in cleanup (on an
application side).
##########
src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java:
##########
@@ -35,6 +34,8 @@
public class ThreadPoolExecutorCleaningThreadLocals extends ThreadPoolExecutor
{
private final ThreadLocalChangeListener listener;
+ private long cleanupCounter = 0;
Review Comment:
As this might be called concurrently, I would use an atomic type here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
