For 1, JNDI lookup that is peformed by
resource injection of a ManagedThreadFactory, I can see where
it’s a bit unclear about the thread performing the
lookup/creation of the ManagedThreadFactory. It would be a
good idea for the spec to elaborate more on its requirements
for this scenario. I searched the TCK to identify whether it
is obtaining a ManagedThreadFactory by resource injection
anywhere. None of the newly added test are doing this, so it
is not a cause of the errors you are running into. I did find
it being done in 2 of the pre-existing TCK tests from
Concurrency 2.0 (I checked the old platform TCK to be sure a
change to use resource injection wasn’t introduced during
porting – it wasn’t). Neither of these make any assertions
about context,
@Local(ContextPropagateInterface.class)
@Stateless
public class
ContextPropagateBean implements ContextPropagateInterface {
@Resource(lookup =
"java:comp/DefaultManagedThreadFactory")
private ManagedThreadFactory
threadFactory;
and
@WebServlet("/testServlet")
public class TestServlet
extends HttpServlet {
@Resource(lookup =
"java:comp/DefaultManagedThreadFactory")
private ManagedThreadFactory
factory;
(The latter is renamed form TestServlet to
APIServlet after the port if anyone is looking for the
equivalent in the 3.0 TCK)
The specification document, going back to
1.0, also contains 2 examples with resource injection of
ManagedThreadFactory,
@Resource(name="concurrent/ThreadFactory")
ManagedThreadFactory
threadFactory;
@PostConstruct
public void postConstruct() {
threadPoolExecutor = new
ThreadPoolExecutor( 5, 10, 5, TimeUnit.SECONDS, new
ArrayBlockingQueue<Runnable>(10), threadFactory);
}
and
@Resource(name=”concurrent/LoggerThreadFactory”)
ManagedThreadFactory
threadFactory;
I can tell you how Open Liberty attempted
to best follow the intent of the spec in this case, which is
that we capture the identity of the application component into
which the resource is being injected and apply that to threads
created by the ManagedThreadFactory in order to allow the JNDI
lookups on its application component namespace, and we would
be using empty context for most other types because there is
nothing much else on the thread performing the initialization
to capture context from. That was our interpretation of
intent – I’m not claiming that the spec provides sufficient
detail on the resource injection path to require that much of
other implementations.
We could open an issue, copying from the
above information for Concurrency vNext to clarify context
requirements for the resource injection path. Or we could
leave it vendor-specific. Either would be fine with me, but
it doesn’t seem an immediate concern because existing tests
would already be passing and the new tests don’t use this
pattern.
For 2, when multiple threads perform,
ManagedThreadFactory mtf =
InitialContext.doLookup("java:app/concurrent/mtf1");
the specification does not have a
requirement that the same instance be returned from each
lookup. The Open Liberty implementation chose to return a
different instance for each ManagedThreadFactory lookup, which
we did specifically to attach the context information to
enable our own way of meeting the specification requirement of
applying the context of the ManagedThreadFactory creator to
the managed threads. I didn’t realize other implementations
would have limitations against using multiple instances. For
implementations that must return the same instance from
lookups, it seems like this specification requirement would
cause a lot of difficulty, risk, and possibly be
unimplementable. If you are going to file a challenge to the
TCK, I think that is the grounds to do it upon. To use the
exact words from Scott’s email: “Claims that an assertion of
the specification is not sufficiently implementable.”
As far as usage patterns go, the spec is
currently optimized toward a ManagedThreadFactory being used
to back a thread pool. In that case, you want the
predictability of every thread running with the same context,
regardless of whether you happened to get a thread that was
created in response to your current request or to some other
previous request. It’s also highly efficient because there is
little overhead to capturing, establishing, and removing
context from threads – the context just stays on the pooled
threads. The obvious disadvantage is that the container isn’t
managing the thread pool.
But the pattern you are thinking about with
direct usage of newThread makes sense as well. An idea for a
future spec version could be to allow either and make the
behavior configurable on ManagedThreadFactoryDefinition, but
that probably only works for you if your implementation would
have a way of supporting both behaviors. We can leave that
idea to future discussion and open an issue for it if it is of
any interest.
Hello everybody, I
have an issue, what is the expected behavior of
ManagedThreadFactory, particularly when the calling
thread's context should be saved; my expectation was that
it is similar to ManagedExecutorService. I did PR and got
answer
|
This
Message Is From an External Sender
|
|
This
message came from outside your
organization.
|
|
|
Hello everybody, I have an issue, what is
the expected behavior of ManagedThreadFactory, particularly
when the calling thread's context should be saved; my
expectation was that it is similar to ManagedExecutorService.
I did PR and got answer from Nathan, that it should happen on
JNDI lookup(): https://github.com/jakartaee/concurrency/pull/212
The javadoc for ManagedThreadFactorysays this:
https://github.com/jakartaee/concurrency/blob/1acd0022c9955f2a6c7da5604ac9b0e09712fd31/api/src/main/java/jakarta/enterprise/concurrent/ManagedThreadFactory.java#L40
* {@link ThreadFactory#newThread(Runnable)} method * will *run
with the **application component context of the component
instance* * that created (looked-up) this ManagedThreadFactory
instance.
* The Runnable task that is allocated to the new thread using
the I understand it this way -- when executed, the java:comp
is set to the EJB doing the JNDI lookup. But saving thread
context at this moment is strange to me. Looking up in JNDI
simply returns the object, not initializing it. I have two
questions, when it is expected to save the calling thread: 1)
@Stateless class A { @Resource("java:app/concurrent/MTF1")
private ManagedThreadFactory mtf1; public void f() {
mtf1.newThread(() -> {}).start(); } } Here, the
mtf1 field is initiated during creation of the EJB object A,
which is definitely wrong thread for context save. 2)
@Stateless class B { public void f1() {
ManagedThreadFactory mtf =
InitialContext.doLookup("java:app/concurrent/mtf1");
mtf.newThread(() -> {}).start(); } public void f2()
{ ManagedThreadFactory mtf =
InitialContext.doLookup("java:app/concurrent/mtf1");
mtf.newThread(() -> {}).start(); } public void f3()
{ ManagedThreadFactory mtf =
InitialContext.doLookup("java:app/concurrent/mtf1");
mtf.newThread(() -> {}).start(); } } All 3 methods
return the same object. If they have to store the context
after doLookup(), they would need to store it in ThreadLocal -
correct? Also, the factories must be notified, that they were
looked-up, right? The easiest explanation seems to me to be
most obvious behavior -- save the context during newThread and
stored in the ManagedThread. It is consistent across all
usages (lookup, even in multiple invocations, injection) and
also consistent with ManagedExecutorService. The same is with
ForkJoinPool using ManagedThreadFactory, but let's solve
ManagedThreadFactory itself first. Thank you for your opinion
or explanation, where I'm doing a mistake. Petr
