Great! :) 2016-07-06 11:48 GMT+02:00 John D. Ament <johndam...@apache.org>:
> Yeah, tests weren't working :-) > > But good news, the Weld2 profile is working now. I suppose that this issue > was weld specific. > > John > > On Wed, Jul 6, 2016 at 4:40 AM Thomas Andraschko < > andraschko.tho...@gmail.com> wrote: > > > Oh sorry, you already made it ApplicationScoped with your second commit > :) > > > > 2016-07-06 10:40 GMT+02:00 Thomas Andraschko < > andraschko.tho...@gmail.com > > >: > > > > > Hi John, > > > > > > thanks for fixing it. > > > The current solution will be very slow and we should make > > > DelegateManualInvocationHandler and InterceptManualInvocationHandler > > > ApplicationScoped. > > > It would be still great if you could compare the performance e.g. for > > > 10.000 calls with our old code (with static) vs > > > ApplicationScoped+BeanProvider. > > > > > > 2016-07-06 4:12 GMT+02:00 John D. Ament <johndam...@apache.org>: > > > > > >> I ended up pushing a second commit which I think will make things > > better. > > >> But still not 100% sure. The problem is that the static invocation > > >> handler > > >> was holding a reference to InterceptorLookup, which gets lost between > > >> container boots (or even, if there are multiple bean managers around > for > > >> some reason). > > >> > > >> On Tue, Jul 5, 2016 at 9:42 PM John D. Ament <john.d.am...@gmail.com> > > >> wrote: > > >> > > >> > Hi @Thomas and others > > >> > > > >> > This was a tricky one but I think I got it. Don't have a good test > > for > > >> it > > >> > yet. > > >> > > > >> > Basically, in the case of web profile tests, we didn't see this as > an > > >> > issue as the classloader gets cleared, so its not a problem. > > >> > > > >> > However, if you test proxy based classes in an SE environment, the > > >> static > > >> > reference to the contextual instance may be loaded incorrectly. In > > >> those > > >> > situations, we need to make sure its cleared. > > >> > > > >> > I removed the static reference. I'm concerned about the performance > > >> > implications. Do you see any reason to not use an app scoped bean > > here > > >> > instead of dependent? I wouldn't consider this done, as the > dependent > > >> bean > > >> > still leaks. > > >> > > > >> > John > > >> > > > >> > > > >> > On Tue, Jul 5, 2016 at 9:37 PM <johndam...@apache.org> wrote: > > >> > > > >> >> Repository: deltaspike > > >> >> Updated Branches: > > >> >> refs/heads/master 81b390e29 -> bc8c82e5b > > >> >> > > >> >> > > >> >> DELTASPIKE-1179 Don't use a static reference to a contextual bean, > in > > >> the > > >> >> case of beans being reloaded by multiple deployments. > > >> >> > > >> >> > > >> >> Project: http://git-wip-us.apache.org/repos/asf/deltaspike/repo > > >> >> Commit: > > >> http://git-wip-us.apache.org/repos/asf/deltaspike/commit/bc8c82e5 > > >> >> Tree: > > http://git-wip-us.apache.org/repos/asf/deltaspike/tree/bc8c82e5 > > >> >> Diff: > > http://git-wip-us.apache.org/repos/asf/deltaspike/diff/bc8c82e5 > > >> >> > > >> >> Branch: refs/heads/master > > >> >> Commit: bc8c82e5b68fb1d888fa6bac7494ed9870dca80d > > >> >> Parents: 81b390e > > >> >> Author: John D. Ament <johndam...@apache.org> > > >> >> Authored: Tue Jul 5 21:36:41 2016 -0400 > > >> >> Committer: John D. Ament <johndam...@apache.org> > > >> >> Committed: Tue Jul 5 21:37:07 2016 -0400 > > >> >> > > >> >> > > ---------------------------------------------------------------------- > > >> >> .../proxy/impl/invocation/DelegateManualInvocationHandler.java | 6 > > >> +++--- > > >> >> .../impl/invocation/InterceptManualInvocationHandler.java | 6 > > >> +++--- > > >> >> .../apache/deltaspike/test/testcontrol/uc003/TestSuite.java | 3 > > --- > > >> >> 3 files changed, 6 insertions(+), 9 deletions(-) > > >> >> > > ---------------------------------------------------------------------- > > >> >> > > >> >> > > >> >> > > >> >> > > >> > > > http://git-wip-us.apache.org/repos/asf/deltaspike/blob/bc8c82e5/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/DelegateManualInvocationHandler.java > > >> >> > > ---------------------------------------------------------------------- > > >> >> diff --git > > >> >> > > >> > > > a/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/DelegateManualInvocationHandler.java > > >> >> > > >> > > > b/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/DelegateManualInvocationHandler.java > > >> >> index d247253..a4911f7 100644 > > >> >> --- > > >> >> > > >> > > > a/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/DelegateManualInvocationHandler.java > > >> >> +++ > > >> >> > > >> > > > b/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/DelegateManualInvocationHandler.java > > >> >> @@ -18,6 +18,7 @@ > > >> >> */ > > >> >> package org.apache.deltaspike.proxy.impl.invocation; > > >> >> > > >> >> +import org.apache.deltaspike.core.api.provider.BeanProvider; > > >> >> import org.apache.deltaspike.proxy.spi.DeltaSpikeProxy; > > >> >> > > >> >> import java.lang.reflect.InvocationHandler; > > >> >> @@ -32,11 +33,10 @@ import javax.enterprise.inject.Typed; > > >> >> @Typed > > >> >> public class DelegateManualInvocationHandler extends > > >> >> AbstractManualInvocationHandler > > >> >> { > > >> >> - private static final DelegateManualInvocationHandler INSTANCE > = > > >> new > > >> >> DelegateManualInvocationHandler(); > > >> >> - > > >> >> public static Object staticInvoke(Object proxy, Method method, > > >> >> Object[] parameters) throws Throwable > > >> >> { > > >> >> - return INSTANCE.invoke(proxy, method, parameters); > > >> >> + DelegateManualInvocationHandler handler = > > >> >> > > >> > > > BeanProvider.getContextualReference(DelegateManualInvocationHandler.class); > > >> >> + return handler.invoke(proxy, method, parameters); > > >> >> } > > >> >> > > >> >> @Override > > >> >> > > >> >> > > >> >> > > >> > > > http://git-wip-us.apache.org/repos/asf/deltaspike/blob/bc8c82e5/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/InterceptManualInvocationHandler.java > > >> >> > > ---------------------------------------------------------------------- > > >> >> diff --git > > >> >> > > >> > > > a/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/InterceptManualInvocationHandler.java > > >> >> > > >> > > > b/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/InterceptManualInvocationHandler.java > > >> >> index 17e23ae..b4f7d7a 100644 > > >> >> --- > > >> >> > > >> > > > a/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/InterceptManualInvocationHandler.java > > >> >> +++ > > >> >> > > >> > > > b/deltaspike/modules/proxy/impl-asm5/src/main/java/org/apache/deltaspike/proxy/impl/invocation/InterceptManualInvocationHandler.java > > >> >> @@ -18,6 +18,7 @@ > > >> >> */ > > >> >> package org.apache.deltaspike.proxy.impl.invocation; > > >> >> > > >> >> +import org.apache.deltaspike.core.api.provider.BeanProvider; > > >> >> import org.apache.deltaspike.proxy.api.DeltaSpikeProxyFactory; > > >> >> > > >> >> import java.lang.reflect.InvocationTargetException; > > >> >> @@ -31,11 +32,10 @@ import javax.enterprise.inject.Typed; > > >> >> @Typed > > >> >> public class InterceptManualInvocationHandler extends > > >> >> AbstractManualInvocationHandler > > >> >> { > > >> >> - private static final InterceptManualInvocationHandler > INSTANCE = > > >> new > > >> >> InterceptManualInvocationHandler(); > > >> >> - > > >> >> public static Object staticInvoke(Object proxy, Method method, > > >> >> Object[] parameters) throws Throwable > > >> >> { > > >> >> - return INSTANCE.invoke(proxy, method, parameters); > > >> >> + InterceptManualInvocationHandler handler = > > >> >> > > >> > > > BeanProvider.getContextualReference(InterceptManualInvocationHandler.class); > > >> >> + return handler.invoke(proxy, method, parameters); > > >> >> } > > >> >> > > >> >> @Override > > >> >> > > >> >> > > >> >> > > >> > > > http://git-wip-us.apache.org/repos/asf/deltaspike/blob/bc8c82e5/deltaspike/modules/test-control/impl/src/test/java/org/apache/deltaspike/test/testcontrol/uc003/TestSuite.java > > >> >> > > ---------------------------------------------------------------------- > > >> >> diff --git > > >> >> > > >> > > > a/deltaspike/modules/test-control/impl/src/test/java/org/apache/deltaspike/test/testcontrol/uc003/TestSuite.java > > >> >> > > >> > > > b/deltaspike/modules/test-control/impl/src/test/java/org/apache/deltaspike/test/testcontrol/uc003/TestSuite.java > > >> >> index 65e5015..5d26b70 100644 > > >> >> --- > > >> >> > > >> > > > a/deltaspike/modules/test-control/impl/src/test/java/org/apache/deltaspike/test/testcontrol/uc003/TestSuite.java > > >> >> +++ > > >> >> > > >> > > > b/deltaspike/modules/test-control/impl/src/test/java/org/apache/deltaspike/test/testcontrol/uc003/TestSuite.java > > >> >> @@ -29,9 +29,6 @@ import > > >> >> > org.apache.deltaspike.test.testcontrol.shared.ApplicationScopedBean; > > >> >> > > >> >> //Usually NOT needed! Currently only needed due to our > > >> arquillian-setup > > >> >> @Category(SeCategory.class) > > >> >> - > > >> >> - > > >> >> - > > >> >> @RunWith(CdiTestSuiteRunner.class) //starts container once (for > the > > >> >> whole suite) > > >> >> @Suite.SuiteClasses({ > > >> >> RequestAndSessionScopePerTestMethodTest.class, > > >> >> > > >> >> > > >> > > > > > > > > >