Hi,

On Wed, May 11, 2016 at 10:54 AM, Martin Kouba <mkouba@redhat.com> wrote:
Hi Arjan,

the test attached to WELD-2148 is wrong. We can not inspect an actual SLSB instance. The only thing we could test is that Instance.destroy() does not throw UnsupportedException for SLSB CDI reference.

I see, thanks for the clarification!

Going forward, would it not be convenient if an Instance was AutoCloseable, so that one could do:

try (Instance<Foo> fooInstance = CDI.current().select(Foo.class)) {
    Foo foo = fooInstance.get();
    foo.bar();
}

Kind regards,
Arjan Tijms



 

Martin

Dne 11.5.2016 v 10:40 arjan tijms napsal(a):
Hi Martin,

Thanks for the swift action and the reference. I do have one more
question looking at the test that was added. It now uses this SLSB:

@Stateless
public class SLSessionBean {

     public void ping(){
     }

     static final AtomicBoolean DESTROYED = new AtomicBoolean();

     @PreDestroy
     public void destroy() {
         DESTROYED.set(true);
     }
}

The assertion in the test is that the (a?) SLSB is actually destroyed,
but wasn't the idea that only the internal reference is destroyed, and
the bean just stays in the pool?

Here it looks like the code intends to destroy a random SLSB instance
from the pool. (random, since I guess an internal reference doesn't
stick to the same actual instance of a SLSB, otherwise you would get
stateful like semantics).

Or did I miss something?

Kind regards,
Arjan Tijms




On Wed, May 11, 2016 at 9:11 AM, Martin Kouba <mkouba@redhat.com
<mailto:mkouba@redhat.com>> wrote:

    Hi Arjan,

    I believe it's a Weld bug - you should be able to use
    Instance.destroy() to discard an internal SLSB proxy. See also
    "Lifecycle of stateless and singleton session beans" [1]. Tomas
    Remes created WELD-2148 to track this issue [2].

    Also the "leak" is an expected behaviour. See for example WELD-920
    [3] discussion.

    Martin

    [1]
    http://docs.jboss.org/cdi/spec/1.2/cdi-spec.html#stateless_lifecycle

    [2]
    https://issues.jboss.org/browse/WELD-2148

    [3]
    https://issues.jboss.org/browse/WELD-920

    Dne 10.5.2016 v 17:11 arjan tijms napsal(a):

        Hi,

        Given a simple @Stateless bean:

        @Stateless
        public class Foo {
              public void bar() {}
        }

        Then requesting an instance of this via CDI as follows:

        Foo foo = CDI.current().select(Foo.class).get();

        Causes a lot of leaked proxy instances (at least on Weld). Now
        what I
        guess needs to be done is destroying the proxy, taking Antoine's
        answer
        here as a lead:
        http://stackoverflow.com/questions/28767536/scope-of-stateless-bean

        Only the following throws an UnsupportedOperationException (on Weld
        2.3.2, haven't tested OWB yet)

        Instance<Foo> fooInstance =CDI.current().select(Foo.class);
        Foo foo = fooInstance.get();
        foo.bar();
        fooInstance.destroy(foo);

        The question is, is this how it's supposed to be done via the spec?

        Implementation wise, what happens in Weld is that the CDI/EJB proxy
        (com.test.Foo$Proxy$_$$_Weld$EnterpriseProxy$) in the following code
        doesn't hit the check for a dependent instance (comments in capitals
        added by me):


              public void destroy(T instance) {
                  Preconditions.checkNotNull(instance);

                  // check if this is a proxy of a normal-scoped bean
                  if (instance instanceof ProxyObject) {

                      // THIS BRANCH IS TAKEN FOR CDI/EJB PROXY

                      ProxyObject proxy = (ProxyObject) instance;
                      if (proxy.getHandler() instanceof
        ProxyMethodHandler) {
                          ProxyMethodHandler handler = (ProxyMethodHandler)
        proxy.getHandler();
                          Bean<?> bean = handler.getBean();
                          Context context =
        getBeanManager().getContext(bean.getScope());
                          if (context instanceof AlterableContext) {
                              AlterableContext alterableContext =
        (AlterableContext) context;

                              // CONTEXT IS A DEPENDENTCONTEXTIMPL THAT
        THROWS
                              // UnsupportedOperationException FROM ITS
        DESTROY()
        METHOD
                              alterableContext.destroy(bean);
                              return;
                          } else {
                              throw
        BeanLogger.LOG.destroyUnsupported(context);
                          }
                      }
                  }

                  // check if this is a dependent instance
                  CreationalContext<? super T> ctx = getCreationalContext();
                  if (ctx instanceof WeldCreationalContext<?>) {
                      WeldCreationalContext<? super T> weldCtx = cast(ctx);

                      // PROXY REFERENCES ARE KEPT HERE IN A
                      // "dependentInstances" LIST, AND WOULD BE CLEARED
        HERE
                      // BUT THIS CODE IS NEVER REACHED
                      weldCtx.destroyDependentInstance(instance);
                  }
              }

        Now I wonder, am I doing something wrong (according to the CDI
        spec), or
        could this be a bug in the Weld code?

        Kind regards,
        Arjan Tijms


        _______________________________________________
        cdi-dev mailing list
        cdi-dev@lists.jboss.org <mailto:cdi-dev@lists.jboss.org>
        https://lists.jboss.org/mailman/listinfo/cdi-dev

        Note that for all code provided on this list, the provider
        licenses the code under the Apache License, Version 2
        (http://www.apache.org/licenses/LICENSE-2.0.html). For all other
        ideas provided on this list, the provider waives all patent and
        other intellectual property rights inherent in such information.


    --
    Martin Kouba
    Software Engineer
    Red Hat, Czech Republic



--
Martin Kouba
Software Engineer
Red Hat, Czech Republic