What is the correct usage of WELD's built-in HttpRequestContext bean?

[chrisjr]

Hi,

I have implemented a CDI bean with a @Produces method that extracts query parameter "traceId" from any/every HTTP request. This @Produces method is also likely to be called outside of request scope (by beans running in user-created threads), and so must return null in this case instead of throwing an exception. I am using the built-in HttpRequestContext bean to determine whether or not request scope is active:

@ApplicationScoped
public class TraceProducer {

    private final HttpRequestContext context;

    @Inject
    TraceProducer(HttpRequestContext context) {
        this.context = context;
    }

    @Produces
    @Trace
    String getTrace(HttpServletRequest request) {
        String traceId = null;

        if (context.isActive()) {
            traceId = request.getParameter("traceId");
            if (traceId == null) {
                traceId = generateTraceId();
            }
        }

        return traceId;
    }

    private static String generateTraceId() {
        return ...; // generates a random unique String
    }

}

Is this a legal / intended use of HttpRequestContext, please? My original implementation of this bean was @Dependent scoped instead of @ApplicationScoped, but then I noticed that the heap has begun accumulating CreationalContext objects associated with HttpRequestContextImpl with each HTTP request. I have also noticed that HttpRequestContext is specific to WELD rather than being an interface defined by CDI. Both of these things make me suspect that I am meddling with low-level WELDy things that aren't supposed to be exposed inside applications. However, using HttpRequestContext does seem like a neater solution to my problem that trying to catch a "request scope is not active" exception thrown by the built-in HttpServletRequest bean.

The above code does appear to work as intended - at the moment. But I'd hate to think that I was ultimately relying on some undocumented behaviour that could disappear one day. Does anyone have any thoughts on this approach, please?

Thanks,

Chris

[tremes]

Hi,

I think you can do the same by injecting BeanManager and then evaluating beanManager.getContext(RequestScoped.class).isActive() and you are using only CDI API in this case.

[chrisjr]

Actually, my understanding of the memory leak that I saw wasn't correct. The objects that were leaking are:

- org.jboss.weld.context.CreationalContextImpl

- org.jboss.weld.context.SerializableContextualInstanceImpl

- org.jboss.weld.context.SerializableContextualFactory$SerializableContextualHolder

- org.jboss.weld.context.SerializableContextualFactory$PassivationCapableSerializableBean

And switching to @ApplicationScoped from @Dependent isn't what fixed it after all. The leak was actually caused by this @Produces method signature:

@Produces
@Trace
String getTrace(HttpRequestContext context, HttpServletRequest request) {
    ...
}

with HttpRequestContext as a parameter!

I wasn't aware of any restrictions for where CDI beans can be injected safely. Could this be a WELD bug? I am using WELD 2.2.14.Final.

[mkouba]

Hi Chris, what bean is the consumer of your getTrace() producer? The thing is your producer method is @Dependent and as such is bound to the lifecycle of the consumer. Moreover, HttpRequestContext is also @Dependent and so it's a dependent object of the producer method bean instance that is being produced.

[chrisjr]

Hi Tomas,

I tried your suggestion, but unfortunately BeanManager.getContext(RequestScoped.class) also throws an exception if the request scope is not active.

org.jboss.weld.context.ContextNotActiveException: WELD-001303: No active contexts for scope type javax.enterprise.context.RequestScoped
    at org.jboss.weld.manager.BeanManagerImpl.getContext(BeanManagerImpl.java:708) ~[weld-core-2.2.14.Final.jar:2015-06-24 15:18]
    at org.jboss.weld.util.ForwardingBeanManager.getContext(ForwardingBeanManager.java:181) ~[weld-core-2.2.14.Final.jar:2015-06-24 15:18]

Cheers,

Chris

[chrisjr]

Hi Martin,

The @Produces method is consumed by a @Dependent bean that is ultimately attached to 3 different @ApplicationScoped beans. But are you saying that all of @Produces method's injected parameters are being bound up in the produced bean's @Dependent scope? In my case, I was only passing HttpRequestContext into the method so that I could call its isActive() method, and since it could never be attached to the String result I had assumed that WELD would dispose of it completely afterwards.

Cheers,

Chris

[chrisjr]

This is one of those times when I really, really wish that WELD could just inject a POJO. Is there any reason why WELD would need to keep a CreationalContext lying around when producing instances of String? Or of Integer / int, Long / long, Boolean / boolean etc? Particularly if these CreationalContexts are going to clump together silently...

[mkouba]

But are you saying that all of @Produces method's injected parameters are being bound up in the produced bean's @Dependent scope?

Yes, exactly. However, you can annotate the injection point with @TransientReference and the contextual instance will be destroyed when the invocation completes. See also 6.4.1. Dependent objects and 6.4.2. Destruction of objects with scope @Dependent (since CDI 1.1).

[chrisjr]

Ah, thanks. I was unaware of @TransientReference. However, now that I am aware of it, I find myself irritated to discover that I can't annotate a @Produces method with it. I would much rather annotate a @Produces String method as @TransientReference instead of all of its injection points.

[chrisjr]

Out of interest, I see that there is an interesting "optimisation" mentioned in WELD-1076:

Alternatively, we should not use creational context to store dependent instances that do not define @PreDestroy callbacks nor are they intercepted by a @PreDestroy interceptor.

Doesn't this optimisation also apply to dependent beans from @Produces methods that don't have corresponding @Disposes methods? And it not, then could it?

[mkouba]

However, now that I am aware of it, I find myself irritated to discover that I can't annotate a @Produces method with it. I would much rather annotate a @Produces String method as @TransientReference instead of all of its injection points.

Hm, I don't think this would make much sense. What would be the purpose? To define that all parameter injection points should be transient references?

[chrisjr]

What would be the purpose? To define that all parameter injection points should be transient references?

That's what I would be using it for, yes. But is there any reason that you would want to keep a contextual reference to a String bean (or a bean of any other built-in Java type) apart from when you've defined an explicit @Disposes method for it?

[chrisjr]

One thing that I forgot to mention is that I always invoke TraceProducer via an Instance:

@Trace
private Instance<String> tracer;

public Thingy create() {
    return new Thing(tracer.get());
}

So each @Trace String bean should belong to the Instance<>, which in turn is always bound to an @ApplicationScoped bean. And yet I never saw these Strings being leaked, which implies that WELD was already releasing their contexts. And yet the HttpRequestContext beans - which were bound to the String contexts via the @Produces method - were not released.

Oh dear...

[mkouba]

Doesn't this optimisation also apply to dependent beans from @Produces methods that don't have corresponding @Disposes methods? And it not, then could it?

Yes, this optimization should be applied as well. There are some tests in the Weld test suite.

And yet the HttpRequestContext beans - which were bound to the String contexts via the @Produces method - were not released.

This is odd. Could you post the latest source? In the first snippet there's HttpRequestContext injected into the TraceProducer.

[chrisjr]

I have always used the TraceProducer via @Trace Instance<String>.get(), and I have never called Instance<String>.destroy() afterwards. WELD is apparently "optimising away" the contextual instance for @Trace String so that I don't leak memory - this is fine. However, when I implemented the @Produces method like this:

@Produces
@Trace
String getTrace(HttpRequestContext context, HttpServletRequest request) {
    return ...;
}

then the contextual instance for HttpRequestContext was still leaked. My updated understanding of WELD now suggests that either the contextual instances for both @Trace String and HttpRequestContext should be leaked, or neither should be. I would obviously prefer the "neither" scenario...

Regardless, I have now reimplemented TraceProducer as:

private final HttpRequestContext context;

@Inject
TraceProducer(HttpRequestContext context) {
    this.context = context;
}

@Produces
@Trace
String getTrace(HttpServletRequest request) {
    return ...;
}

and my leak is fixed. (I'm still not calling Instance<String>.destroy() though). So now we're just left with a theoretical discussion over an edge-case with dependent scoped beans.