glimmerveen commented on code in PR #419:
URL: https://github.com/apache/felix-dev/pull/419#discussion_r2086224653
##########
scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java:
##########
@@ -138,6 +142,17 @@ public ComponentRegistry( final ScrConfiguration
scrConfiguration, final ScrLogg
m_componentHoldersByPid = new HashMap<>();
m_componentsById = new HashMap<>();
+ ScheduledThreadPoolExecutor threadPoolExecutor = new
ScheduledThreadPoolExecutor(1, new ThreadFactory()
+ {
+ @Override
+ public Thread newThread(Runnable r)
+ {
+ return new Thread(r, "SCR Component Registry");
+ }
+ });
+ threadPoolExecutor.setKeepAliveTime(10, TimeUnit.SECONDS);
Review Comment:
Not having any timeout would further simply things (ie using one of the
standard factory methods, over manually initializing one of the
ScheduledExecutorService implementations). I opted for this, as wanted to stay
as close to the current behaviour, and thus wanted to retain the cleanup of the
thread, with one change that the thread is not cleaned-up immediately but after
some period of time.
I can make that period Thread timeout configurable, similar to how the
changecount timeout is configurable, and even include a way to never have a
timeout, but with the latter as well, I also see this adding some complexity as
well. For instance, should there be some kind of check on the Thread timeout vs
the changecount timeout?
Coming back to your initial suggestion, would something like this work:
`threadPoolExecutor.setKeepAliveTime(Math.max(m_configuration.componentRegistryThreadTimeout(),
m_configuration.serviceChangecountTimeout()*2), TimeUnit.MILLISECONDS);`
So having the thread timeout be 2x the changecount timeout, with a
configurable minimum which is by default 500ms?
--
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]
