Using ThreadLocal in InjectableContext/ManagedContext.

[skku-daniilkim]

Hello,

Is it allowed to use ThreadLocal variables in the InjectableContext/ManagedContext implementations for custom CDI scopes? Will there be any problems? There is a ThreadLocalCurrentContext.java, but it does not seem to be used anywhere.

[mkouba]

So we do try to avoid using ThreadLocals in Quarkus. The ThreadLocalCurrentContext is only used for tests. If you work on a custom non-shared context for a normal scope then try to use the io.quarkus.arc.CurrentContextFactory to create a new io.quarkus.arc.CurrentContext. The Vertx implementation (which is used in most cases) attempts to use the Vertx duplicated context to store the bean instances if possible and falls back to ThreadLocal otherwise.

You can obtain the CurrentContextFactory with Arc.container().getCurrentContextFactory(). There is also the io.quarkus.arc.impl.CurrentManagedContext, an abstract class that handles most of the logic. Finally, you will probably need the io.quarkus.arc.impl.ComputingCacheContextInstances which stores the bean instances in a computing cache.

You should end up with something like the built-in SessionContext.

[mkouba]

No need to aplologize ;-). They're not synonyms. But I don't think that "misleading" is always "intentionally confusing". For me "misleading" means that it can lead someone to a wrong conclusion, no matter if intentional or unintentional. In this particular case, the "wrong conclusion" might be that you should extend the AbstractManagedContext if you implement a ManagedContext which is obviously not the case.

Enough of the chatter, I'm pretty sure we'll manage to reach a compromise!

[skku-daniilkim]

One more question, why does InjectableRequestContextController use AtomicBoolean, while a seemingly identical InjectableCommandContext controller from the Weld example (https://github.com/weld/command-context-example/blob/master/src/main/java/org/jboss/weld/examples/command/CommandContextImpl.java) uses raw boolean? Same about the Quarkus' reimplementation of the aforementioned Weld example (https://github.com/quarkusio/quarkus/blob/main/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/cdi/bcextensions/CustomNormalScopeTest.java). Why was the "isActivator" changed to an AtomicBoolean; Is there a case, when this matters?

If we assume that this can be used in multi-threaded environment, then isn't there still a possible edge case when this can break?

quarkus/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/cdi/bcextensions/CustomNormalScopeTest.java

Lines 251 to 260 in dcef943

	public boolean activate() {
	try {
	beanContainer.getContext(CommandScoped.class);
	return false;
	} catch (ContextNotActiveException e) {
	context.activate();
	activated.set(true);
	return true;
	}
	}

beanContainer.getContext(CommandScoped.class); 
// <-- context gets deactivated here.
return false; 

I apologize if I missed something.

[Ladicek]

The reason for using AtomicBoolean is not concurrent access, that should never happen in case of the request context (or the command context, for that matter). It is context propagation, which can cause the "owning" thread to change over time (but there should still always be just one), so we need to take care of visibility. Also, the compareAndSet() API is just nicer to use :-)

[skku-daniilkim]

In that case, aside from the niceties of compareAndSet, would it be sufficient to just use volatile?

[Ladicek]

Yes, that should be enough.