You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by rm...@apache.org on 2013/12/03 18:57:19 UTC
svn commit: r1547499 -
/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Author: rmannibucau
Date: Tue Dec 3 17:57:19 2013
New Revision: 1547499
URL: http://svn.apache.org/r1547499
Log:
correct hashcode in lazywebappclassloader
Modified:
tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Modified: tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
URL: http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
==============================================================================
--- tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java (original)
+++ tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java Tue Dec 3 17:57:19 2013
@@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
private boolean restarting = false;
private boolean forceStopPhase = Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase", "false"));
private ClassLoaderConfigurer configurer = null;
+ private final int hashCode;
public LazyStopWebappClassLoader() {
- construct();
+ hashCode = construct();
}
public LazyStopWebappClassLoader(final ClassLoader parent) {
super(parent);
- construct();
+ hashCode = construct();
}
- private void construct() {
+ private int construct() {
setDelegate(isDelegate());
configurer = INIT_CONFIGURER.get();
+ return super.hashCode();
}
@Override
@@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
}
@Override
- public int hashCode() { // could be improved a bit adding the host and ensuring contextName != null, an alternative is getURLs() but it is longer
- final String name = getContextName();
- if (name != null) {
- return name.hashCode();
- }
- return super.hashCode();
+ public int hashCode() {
+ return hashCode;
}
@Override
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Thiago Veronezi <th...@veronezi.org>.
Thanks for your feedback Romain! Indeed this is not an easy thing to test.
The build is broken now; and it looks like this is the reason.
I will rollback the changes while I'm unable to create an unit test for it.
[]s,
Thiago.
On Wed, Dec 4, 2013 at 2:38 AM, Romain Manni-Bucau <rm...@gmail.com>wrote:
> Marker = key in a hasmap or sthg like it.
>
> The point is we use kind of fake loader delegating to real loaders. So
> hashCode is effectively equals but equals shouldnt return true from a
> strict perspective but if it doesnt do it we are not able to use cdi
> correctly (excepted luck or particular case) and a lot of ee libs are
> broken
>
> To try to do a test I think you need to execute both from a webapp and from
> a jaxws request. But when the bug was reported it was very hard to find and
> it was in a complicated case so not sure that's trivial to do, we have good
> fallbacks for simple cases
> Le 4 déc. 2013 03:03, "Thiago Veronezi" <th...@veronezi.org> a écrit :
>
> > Hmmmm, ok. I guess I start to understand what the problem is. If I'm
> right,
> > we already have a buggy feature. Bear with me.
> >
> > None of the parents of LazyStopWebappClassLoader implements "hashCode",
> > therefore our current "LazyStopWebappClassLoader#equals" implementation
> (at
> > the least the one just before my last commit) is based on the result of
> > "Object.hashCode()". According the the "java api", "Object#hashCode" is
> > typically implemented by converting the internal address of the object
> into
> > an integer.
> >
> > Furthermore, the general contract of hashCode is (also java api):
> > * Whenever it is invoked on the same object more than once during an
> > execution of a Java application, the hashCode method must consistently
> > return the same integer, provided no information used in equals
> comparisons
> > on the object is modified. This integer need not remain consistent from
> one
> > execution of an application to another execution of the same application.
> > * If two objects are equal according to the equals(Object) method, then
> > calling the hashCode method on each of the two objects must produce the
> > same integer result.
> > * It is not required that if two objects are unequal according to the
> > equals(java.lang.Object) method, then calling the hashCode method on each
> > of the two objects must produce distinct integer results. However, the
> > programmer should be aware that producing distinct integer results for
> > unequal objects may improve the performance of hash tables.
> >
> >
> > So, in our case...
> >
> > LazyStopWebappClassLoader.java
> > ******************************************************
> > public boolean equals(final Object other) {
> > return other != null && ClassLoader.class.isInstance(other) &&
> > hashCode() == other.hashCode();
> > }
> > public int hashCode() {
> > return super.hashCode();
> > }
> > ******************************************************
> >
> > If this.hashCode() is equal to other.hashCode(), that means that "this ==
> > other". In other words, "this" is the same instance (same memory address)
> > represented by "other". In this case we have the same result as we would
> > have by using the "Object" implementation of both methods.
> >
> > Object.class
> > ******************************************************
> > public boolean equals(Object obj) {
> > return (this == obj);
> > }
> > public native int hashCode();
> > ******************************************************
> >
> >
> >
> > Why I think we have a bug? In both implementations, we would never have
> two
> > equal classloader objects if both aren't exactly the same instance
> because
> > in both implementations we use "Object.hashCode" to compare "this" and
> > "other". Am I right?
> >
> > >>we and libs like deltaspike use classloader as marker
> > I didn't get what you mean by "marker". How does it work?
> >
> > Anyway, the build was successful and all my own applications still run
> > perfectly. Can you give me some direction on how to create a unit test
> that
> > validates it?
> >
> > []s,
> > Thiago.
> >
> >
> >
> >
> > On Tue, Dec 3, 2013 at 6:48 PM, Romain Manni-Bucau <
> rmannibucau@gmail.com
> > >wrote:
> >
> > > The more obvious issue comes from the fact we and libs like deltaspike
> > use
> > > classloader as marker so if both are not equals when needed it doesnt
> > work
> > > (we dont find cdi context ror instance)
> > > Le 3 déc. 2013 23:34, "Thiago Veronezi" <th...@veronezi.org> a écrit
> :
> > >
> > > > >>If you have time to add tests it would be great
> > > > I will test it a little bit further.
> > > >
> > > > But I didn't quite get the problem yet... sorry! :)
> > > >
> > > >
> > > > This is our implementation (without the cache)...
> > > >
> > > >
> > >
> >
> *****************************************************************************************************
> > > > public boolean equals(final Object other) {
> > > > return other != null && ClassLoader.class.isInstance(other) &&
> > > > hashCode() == other.hashCode();
> > > > }
> > > >
> > > > public int hashCode() {
> > > > return super.hashCode();
> > > > }
> > > >
> > > >
> > >
> >
> *****************************************************************************************************
> > > >
> > > > This is the default implementation...
> > > >
> > > >
> > >
> >
> *****************************************************************************************************
> > > > public boolean equals(Object obj) {
> > > > return (this == obj);
> > > > }
> > > >
> > > > public native int hashCode();
> > > >
> > > >
> > >
> >
> *****************************************************************************************************
> > > >
> > > > The default method will return the same integer if "this == other";
> > and
> > > we
> > > > are basically comparing the result of both hashCode() calls. Isn't it
> > the
> > > > same thing?
> > > >
> > > > []s,
> > > > Thiago.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > On Tue, Dec 3, 2013 at 5:11 PM, Romain Manni-Bucau <
> > > rmannibucau@gmail.com
> > > > >wrote:
> > > >
> > > > > well, the issue is without doing it the classloader used for cxf
> > > > > (
> > > > >
> > > >
> > >
> >
> http://svn.apache.org/repos/asf/tomee/tomee/trunk/server/openejb-cxf-transport/src/main/java/org/apache/openejb/server/cxf/transport/util/CxfContainerClassLoader.java
> > > > > )
> > > > > and the webapp loader are different. I'm 100% sure that's a
> > regression
> > > > > even if build passes and we can't keep it.
> > > > >
> > > > > This makes equals not symmetric so using JVM equals/hashCode is the
> > > > > best we can do.
> > > > >
> > > > > If you have time to add tests it would be great, the issue can come
> > > > > from client/server and jaxws/jaxrs. It can cause linkagerror,
> > > > > classcast or even prevent deployment.
> > > > > Romain Manni-Bucau
> > > > > Twitter: @rmannibucau
> > > > > Blog: http://rmannibucau.wordpress.com/
> > > > > LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > > > > Github: https://github.com/rmannibucau
> > > > >
> > > > >
> > > > >
> > > > > 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > > > > It didn't locally. It takes ~2 hrs to have an exception or not.
> > > > > > It should happen in the next 1:15 hrs. If anything happens we can
> > > > always
> > > > > > revert it back. :)
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Tue, Dec 3, 2013 at 4:19 PM, Romain Manni-Bucau <
> > > > > rmannibucau@gmail.com>wrote:
> > > > > >
> > > > > >> it does for sure, not sure we have tests btw
> > > > > >> Romain Manni-Bucau
> > > > > >> Twitter: @rmannibucau
> > > > > >> Blog: http://rmannibucau.wordpress.com/
> > > > > >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > > > > >> Github: https://github.com/rmannibucau
> > > > > >>
> > > > > >>
> > > > > >>
> > > > > >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > > > >> > I tested it locally without the methods.
> > > > > >> > Committed the code to see if the build server complains.
> > > > > >> >
> > > > > >> > []s,
> > > > > >> > Thiago.
> > > > > >> >
> > > > > >> >
> > > > > >> >
> > > > > >> > On Tue, Dec 3, 2013 at 1:11 PM, Romain Manni-Bucau <
> > > > > >> rmannibucau@gmail.com>wrote:
> > > > > >> >
> > > > > >> >> hashcode is just a cached impl, equals is mandatory because
> of
> > > the
> > > > > >> >> hack we have to isolate a bit cxf from apps.
> > > > > >> >>
> > > > > >> >> If you fix isolation you can remove it ;).
> > > > > >> >> Romain Manni-Bucau
> > > > > >> >> Twitter: @rmannibucau
> > > > > >> >> Blog: http://rmannibucau.wordpress.com/
> > > > > >> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > > > > >> >> Github: https://github.com/rmannibucau
> > > > > >> >>
> > > > > >> >>
> > > > > >> >>
> > > > > >> >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > > > >> >> > The equals and hashCode look weird.
> > > > > >> >> >
> > > > > >> >> > ****************************************
> > > > > >> >> > @Override
> > > > > >> >> > public boolean equals(final Object other) {
> > > > > >> >> > return other != null &&
> > > > > ClassLoader.class.isInstance(other) &&
> > > > > >> >> > hashCode() == other.hashCode();
> > > > > >> >> > }
> > > > > >> >> >
> > > > > >> >> > @Override
> > > > > >> >> > public int hashCode() {
> > > > > >> >> > return hashCode;
> > > > > >> >> > }
> > > > > >> >> > ****************************************
> > > > > >> >> >
> > > > > >> >> > I wonder if we need these methods at all. Usually equal
> > > hashCodes
> > > > > does
> > > > > >> >> not
> > > > > >> >> > mean that "equals" is true, right?
> > > > > >> >> >
> > > > > >> >> > []s,
> > > > > >> >> > Thiago.
> > > > > >> >> >
> > > > > >> >> >
> > > > > >> >> >
> > > > > >> >> >
> > > > > >> >> > On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org>
> > > wrote:
> > > > > >> >> >
> > > > > >> >> >> Author: rmannibucau
> > > > > >> >> >> Date: Tue Dec 3 17:57:19 2013
> > > > > >> >> >> New Revision: 1547499
> > > > > >> >> >>
> > > > > >> >> >> URL: http://svn.apache.org/r1547499
> > > > > >> >> >> Log:
> > > > > >> >> >> correct hashcode in lazywebappclassloader
> > > > > >> >> >>
> > > > > >> >> >> Modified:
> > > > > >> >> >>
> > > > > >> >> >>
> > > > > >> >>
> > > > > >>
> > > > >
> > > >
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > > > >> >> >>
> > > > > >> >> >> Modified:
> > > > > >> >> >>
> > > > > >> >>
> > > > > >>
> > > > >
> > > >
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > > > >> >> >> URL:
> > > > > >> >> >>
> > > > > >> >>
> > > > > >>
> > > > >
> > > >
> > >
> >
> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
> > > > > >> >> >>
> > > > > >> >> >>
> > > > > >> >>
> > > > > >>
> > > > >
> > > >
> > >
> >
> ==============================================================================
> > > > > >> >> >> ---
> > > > > >> >> >>
> > > > > >> >>
> > > > > >>
> > > > >
> > > >
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > > > >> >> >> (original)
> > > > > >> >> >> +++
> > > > > >> >> >>
> > > > > >> >>
> > > > > >>
> > > > >
> > > >
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > > > >> >> >> Tue Dec 3 17:57:19 2013
> > > > > >> >> >> @@ -41,19 +41,21 @@ public class
> LazyStopWebappClassLoader e
> > > > > >> >> >> private boolean restarting = false;
> > > > > >> >> >> private boolean forceStopPhase =
> > > > > >> >> >>
> > > > > >> >>
> > > > > >>
> > > > >
> > > >
> > >
> >
> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
> > > > > >> >> >> "false"));
> > > > > >> >> >> private ClassLoaderConfigurer configurer = null;
> > > > > >> >> >> + private final int hashCode;
> > > > > >> >> >>
> > > > > >> >> >> public LazyStopWebappClassLoader() {
> > > > > >> >> >> - construct();
> > > > > >> >> >> + hashCode = construct();
> > > > > >> >> >> }
> > > > > >> >> >>
> > > > > >> >> >> public LazyStopWebappClassLoader(final ClassLoader
> > > parent)
> > > > {
> > > > > >> >> >> super(parent);
> > > > > >> >> >> - construct();
> > > > > >> >> >> + hashCode = construct();
> > > > > >> >> >> }
> > > > > >> >> >>
> > > > > >> >> >> - private void construct() {
> > > > > >> >> >> + private int construct() {
> > > > > >> >> >> setDelegate(isDelegate());
> > > > > >> >> >> configurer = INIT_CONFIGURER.get();
> > > > > >> >> >> + return super.hashCode();
> > > > > >> >> >> }
> > > > > >> >> >>
> > > > > >> >> >> @Override
> > > > > >> >> >> @@ -211,12 +213,8 @@ public class
> LazyStopWebappClassLoader
> > e
> > > > > >> >> >> }
> > > > > >> >> >>
> > > > > >> >> >> @Override
> > > > > >> >> >> - public int hashCode() { // could be improved a bit
> > adding
> > > > the
> > > > > >> host
> > > > > >> >> >> and ensuring contextName != null, an alternative is
> > getURLs()
> > > > but
> > > > > it
> > > > > >> is
> > > > > >> >> >> longer
> > > > > >> >> >> - final String name = getContextName();
> > > > > >> >> >> - if (name != null) {
> > > > > >> >> >> - return name.hashCode();
> > > > > >> >> >> - }
> > > > > >> >> >> - return super.hashCode();
> > > > > >> >> >> + public int hashCode() {
> > > > > >> >> >> + return hashCode;
> > > > > >> >> >> }
> > > > > >> >> >>
> > > > > >> >> >> @Override
> > > > > >> >> >>
> > > > > >> >> >>
> > > > > >> >> >>
> > > > > >> >>
> > > > > >>
> > > > >
> > > >
> > >
> >
>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Marker = key in a hasmap or sthg like it.
The point is we use kind of fake loader delegating to real loaders. So
hashCode is effectively equals but equals shouldnt return true from a
strict perspective but if it doesnt do it we are not able to use cdi
correctly (excepted luck or particular case) and a lot of ee libs are broken
To try to do a test I think you need to execute both from a webapp and from
a jaxws request. But when the bug was reported it was very hard to find and
it was in a complicated case so not sure that's trivial to do, we have good
fallbacks for simple cases
Le 4 déc. 2013 03:03, "Thiago Veronezi" <th...@veronezi.org> a écrit :
> Hmmmm, ok. I guess I start to understand what the problem is. If I'm right,
> we already have a buggy feature. Bear with me.
>
> None of the parents of LazyStopWebappClassLoader implements "hashCode",
> therefore our current "LazyStopWebappClassLoader#equals" implementation (at
> the least the one just before my last commit) is based on the result of
> "Object.hashCode()". According the the "java api", "Object#hashCode" is
> typically implemented by converting the internal address of the object into
> an integer.
>
> Furthermore, the general contract of hashCode is (also java api):
> * Whenever it is invoked on the same object more than once during an
> execution of a Java application, the hashCode method must consistently
> return the same integer, provided no information used in equals comparisons
> on the object is modified. This integer need not remain consistent from one
> execution of an application to another execution of the same application.
> * If two objects are equal according to the equals(Object) method, then
> calling the hashCode method on each of the two objects must produce the
> same integer result.
> * It is not required that if two objects are unequal according to the
> equals(java.lang.Object) method, then calling the hashCode method on each
> of the two objects must produce distinct integer results. However, the
> programmer should be aware that producing distinct integer results for
> unequal objects may improve the performance of hash tables.
>
>
> So, in our case...
>
> LazyStopWebappClassLoader.java
> ******************************************************
> public boolean equals(final Object other) {
> return other != null && ClassLoader.class.isInstance(other) &&
> hashCode() == other.hashCode();
> }
> public int hashCode() {
> return super.hashCode();
> }
> ******************************************************
>
> If this.hashCode() is equal to other.hashCode(), that means that "this ==
> other". In other words, "this" is the same instance (same memory address)
> represented by "other". In this case we have the same result as we would
> have by using the "Object" implementation of both methods.
>
> Object.class
> ******************************************************
> public boolean equals(Object obj) {
> return (this == obj);
> }
> public native int hashCode();
> ******************************************************
>
>
>
> Why I think we have a bug? In both implementations, we would never have two
> equal classloader objects if both aren't exactly the same instance because
> in both implementations we use "Object.hashCode" to compare "this" and
> "other". Am I right?
>
> >>we and libs like deltaspike use classloader as marker
> I didn't get what you mean by "marker". How does it work?
>
> Anyway, the build was successful and all my own applications still run
> perfectly. Can you give me some direction on how to create a unit test that
> validates it?
>
> []s,
> Thiago.
>
>
>
>
> On Tue, Dec 3, 2013 at 6:48 PM, Romain Manni-Bucau <rmannibucau@gmail.com
> >wrote:
>
> > The more obvious issue comes from the fact we and libs like deltaspike
> use
> > classloader as marker so if both are not equals when needed it doesnt
> work
> > (we dont find cdi context ror instance)
> > Le 3 déc. 2013 23:34, "Thiago Veronezi" <th...@veronezi.org> a écrit :
> >
> > > >>If you have time to add tests it would be great
> > > I will test it a little bit further.
> > >
> > > But I didn't quite get the problem yet... sorry! :)
> > >
> > >
> > > This is our implementation (without the cache)...
> > >
> > >
> >
> *****************************************************************************************************
> > > public boolean equals(final Object other) {
> > > return other != null && ClassLoader.class.isInstance(other) &&
> > > hashCode() == other.hashCode();
> > > }
> > >
> > > public int hashCode() {
> > > return super.hashCode();
> > > }
> > >
> > >
> >
> *****************************************************************************************************
> > >
> > > This is the default implementation...
> > >
> > >
> >
> *****************************************************************************************************
> > > public boolean equals(Object obj) {
> > > return (this == obj);
> > > }
> > >
> > > public native int hashCode();
> > >
> > >
> >
> *****************************************************************************************************
> > >
> > > The default method will return the same integer if "this == other";
> and
> > we
> > > are basically comparing the result of both hashCode() calls. Isn't it
> the
> > > same thing?
> > >
> > > []s,
> > > Thiago.
> > >
> > >
> > >
> > >
> > >
> > > On Tue, Dec 3, 2013 at 5:11 PM, Romain Manni-Bucau <
> > rmannibucau@gmail.com
> > > >wrote:
> > >
> > > > well, the issue is without doing it the classloader used for cxf
> > > > (
> > > >
> > >
> >
> http://svn.apache.org/repos/asf/tomee/tomee/trunk/server/openejb-cxf-transport/src/main/java/org/apache/openejb/server/cxf/transport/util/CxfContainerClassLoader.java
> > > > )
> > > > and the webapp loader are different. I'm 100% sure that's a
> regression
> > > > even if build passes and we can't keep it.
> > > >
> > > > This makes equals not symmetric so using JVM equals/hashCode is the
> > > > best we can do.
> > > >
> > > > If you have time to add tests it would be great, the issue can come
> > > > from client/server and jaxws/jaxrs. It can cause linkagerror,
> > > > classcast or even prevent deployment.
> > > > Romain Manni-Bucau
> > > > Twitter: @rmannibucau
> > > > Blog: http://rmannibucau.wordpress.com/
> > > > LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > > > Github: https://github.com/rmannibucau
> > > >
> > > >
> > > >
> > > > 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > > > It didn't locally. It takes ~2 hrs to have an exception or not.
> > > > > It should happen in the next 1:15 hrs. If anything happens we can
> > > always
> > > > > revert it back. :)
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On Tue, Dec 3, 2013 at 4:19 PM, Romain Manni-Bucau <
> > > > rmannibucau@gmail.com>wrote:
> > > > >
> > > > >> it does for sure, not sure we have tests btw
> > > > >> Romain Manni-Bucau
> > > > >> Twitter: @rmannibucau
> > > > >> Blog: http://rmannibucau.wordpress.com/
> > > > >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > > > >> Github: https://github.com/rmannibucau
> > > > >>
> > > > >>
> > > > >>
> > > > >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > > >> > I tested it locally without the methods.
> > > > >> > Committed the code to see if the build server complains.
> > > > >> >
> > > > >> > []s,
> > > > >> > Thiago.
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > On Tue, Dec 3, 2013 at 1:11 PM, Romain Manni-Bucau <
> > > > >> rmannibucau@gmail.com>wrote:
> > > > >> >
> > > > >> >> hashcode is just a cached impl, equals is mandatory because of
> > the
> > > > >> >> hack we have to isolate a bit cxf from apps.
> > > > >> >>
> > > > >> >> If you fix isolation you can remove it ;).
> > > > >> >> Romain Manni-Bucau
> > > > >> >> Twitter: @rmannibucau
> > > > >> >> Blog: http://rmannibucau.wordpress.com/
> > > > >> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > > > >> >> Github: https://github.com/rmannibucau
> > > > >> >>
> > > > >> >>
> > > > >> >>
> > > > >> >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > > >> >> > The equals and hashCode look weird.
> > > > >> >> >
> > > > >> >> > ****************************************
> > > > >> >> > @Override
> > > > >> >> > public boolean equals(final Object other) {
> > > > >> >> > return other != null &&
> > > > ClassLoader.class.isInstance(other) &&
> > > > >> >> > hashCode() == other.hashCode();
> > > > >> >> > }
> > > > >> >> >
> > > > >> >> > @Override
> > > > >> >> > public int hashCode() {
> > > > >> >> > return hashCode;
> > > > >> >> > }
> > > > >> >> > ****************************************
> > > > >> >> >
> > > > >> >> > I wonder if we need these methods at all. Usually equal
> > hashCodes
> > > > does
> > > > >> >> not
> > > > >> >> > mean that "equals" is true, right?
> > > > >> >> >
> > > > >> >> > []s,
> > > > >> >> > Thiago.
> > > > >> >> >
> > > > >> >> >
> > > > >> >> >
> > > > >> >> >
> > > > >> >> > On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org>
> > wrote:
> > > > >> >> >
> > > > >> >> >> Author: rmannibucau
> > > > >> >> >> Date: Tue Dec 3 17:57:19 2013
> > > > >> >> >> New Revision: 1547499
> > > > >> >> >>
> > > > >> >> >> URL: http://svn.apache.org/r1547499
> > > > >> >> >> Log:
> > > > >> >> >> correct hashcode in lazywebappclassloader
> > > > >> >> >>
> > > > >> >> >> Modified:
> > > > >> >> >>
> > > > >> >> >>
> > > > >> >>
> > > > >>
> > > >
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > > >> >> >>
> > > > >> >> >> Modified:
> > > > >> >> >>
> > > > >> >>
> > > > >>
> > > >
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > > >> >> >> URL:
> > > > >> >> >>
> > > > >> >>
> > > > >>
> > > >
> > >
> >
> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
> > > > >> >> >>
> > > > >> >> >>
> > > > >> >>
> > > > >>
> > > >
> > >
> >
> ==============================================================================
> > > > >> >> >> ---
> > > > >> >> >>
> > > > >> >>
> > > > >>
> > > >
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > > >> >> >> (original)
> > > > >> >> >> +++
> > > > >> >> >>
> > > > >> >>
> > > > >>
> > > >
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > > >> >> >> Tue Dec 3 17:57:19 2013
> > > > >> >> >> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
> > > > >> >> >> private boolean restarting = false;
> > > > >> >> >> private boolean forceStopPhase =
> > > > >> >> >>
> > > > >> >>
> > > > >>
> > > >
> > >
> >
> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
> > > > >> >> >> "false"));
> > > > >> >> >> private ClassLoaderConfigurer configurer = null;
> > > > >> >> >> + private final int hashCode;
> > > > >> >> >>
> > > > >> >> >> public LazyStopWebappClassLoader() {
> > > > >> >> >> - construct();
> > > > >> >> >> + hashCode = construct();
> > > > >> >> >> }
> > > > >> >> >>
> > > > >> >> >> public LazyStopWebappClassLoader(final ClassLoader
> > parent)
> > > {
> > > > >> >> >> super(parent);
> > > > >> >> >> - construct();
> > > > >> >> >> + hashCode = construct();
> > > > >> >> >> }
> > > > >> >> >>
> > > > >> >> >> - private void construct() {
> > > > >> >> >> + private int construct() {
> > > > >> >> >> setDelegate(isDelegate());
> > > > >> >> >> configurer = INIT_CONFIGURER.get();
> > > > >> >> >> + return super.hashCode();
> > > > >> >> >> }
> > > > >> >> >>
> > > > >> >> >> @Override
> > > > >> >> >> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader
> e
> > > > >> >> >> }
> > > > >> >> >>
> > > > >> >> >> @Override
> > > > >> >> >> - public int hashCode() { // could be improved a bit
> adding
> > > the
> > > > >> host
> > > > >> >> >> and ensuring contextName != null, an alternative is
> getURLs()
> > > but
> > > > it
> > > > >> is
> > > > >> >> >> longer
> > > > >> >> >> - final String name = getContextName();
> > > > >> >> >> - if (name != null) {
> > > > >> >> >> - return name.hashCode();
> > > > >> >> >> - }
> > > > >> >> >> - return super.hashCode();
> > > > >> >> >> + public int hashCode() {
> > > > >> >> >> + return hashCode;
> > > > >> >> >> }
> > > > >> >> >>
> > > > >> >> >> @Override
> > > > >> >> >>
> > > > >> >> >>
> > > > >> >> >>
> > > > >> >>
> > > > >>
> > > >
> > >
> >
>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Thiago Veronezi <th...@veronezi.org>.
Hmmmm, ok. I guess I start to understand what the problem is. If I'm right,
we already have a buggy feature. Bear with me.
None of the parents of LazyStopWebappClassLoader implements "hashCode",
therefore our current "LazyStopWebappClassLoader#equals" implementation (at
the least the one just before my last commit) is based on the result of
"Object.hashCode()". According the the "java api", "Object#hashCode" is
typically implemented by converting the internal address of the object into
an integer.
Furthermore, the general contract of hashCode is (also java api):
* Whenever it is invoked on the same object more than once during an
execution of a Java application, the hashCode method must consistently
return the same integer, provided no information used in equals comparisons
on the object is modified. This integer need not remain consistent from one
execution of an application to another execution of the same application.
* If two objects are equal according to the equals(Object) method, then
calling the hashCode method on each of the two objects must produce the
same integer result.
* It is not required that if two objects are unequal according to the
equals(java.lang.Object) method, then calling the hashCode method on each
of the two objects must produce distinct integer results. However, the
programmer should be aware that producing distinct integer results for
unequal objects may improve the performance of hash tables.
So, in our case...
LazyStopWebappClassLoader.java
******************************************************
public boolean equals(final Object other) {
return other != null && ClassLoader.class.isInstance(other) &&
hashCode() == other.hashCode();
}
public int hashCode() {
return super.hashCode();
}
******************************************************
If this.hashCode() is equal to other.hashCode(), that means that "this ==
other". In other words, "this" is the same instance (same memory address)
represented by "other". In this case we have the same result as we would
have by using the "Object" implementation of both methods.
Object.class
******************************************************
public boolean equals(Object obj) {
return (this == obj);
}
public native int hashCode();
******************************************************
Why I think we have a bug? In both implementations, we would never have two
equal classloader objects if both aren't exactly the same instance because
in both implementations we use "Object.hashCode" to compare "this" and
"other". Am I right?
>>we and libs like deltaspike use classloader as marker
I didn't get what you mean by "marker". How does it work?
Anyway, the build was successful and all my own applications still run
perfectly. Can you give me some direction on how to create a unit test that
validates it?
[]s,
Thiago.
On Tue, Dec 3, 2013 at 6:48 PM, Romain Manni-Bucau <rm...@gmail.com>wrote:
> The more obvious issue comes from the fact we and libs like deltaspike use
> classloader as marker so if both are not equals when needed it doesnt work
> (we dont find cdi context ror instance)
> Le 3 déc. 2013 23:34, "Thiago Veronezi" <th...@veronezi.org> a écrit :
>
> > >>If you have time to add tests it would be great
> > I will test it a little bit further.
> >
> > But I didn't quite get the problem yet... sorry! :)
> >
> >
> > This is our implementation (without the cache)...
> >
> >
> *****************************************************************************************************
> > public boolean equals(final Object other) {
> > return other != null && ClassLoader.class.isInstance(other) &&
> > hashCode() == other.hashCode();
> > }
> >
> > public int hashCode() {
> > return super.hashCode();
> > }
> >
> >
> *****************************************************************************************************
> >
> > This is the default implementation...
> >
> >
> *****************************************************************************************************
> > public boolean equals(Object obj) {
> > return (this == obj);
> > }
> >
> > public native int hashCode();
> >
> >
> *****************************************************************************************************
> >
> > The default method will return the same integer if "this == other"; and
> we
> > are basically comparing the result of both hashCode() calls. Isn't it the
> > same thing?
> >
> > []s,
> > Thiago.
> >
> >
> >
> >
> >
> > On Tue, Dec 3, 2013 at 5:11 PM, Romain Manni-Bucau <
> rmannibucau@gmail.com
> > >wrote:
> >
> > > well, the issue is without doing it the classloader used for cxf
> > > (
> > >
> >
> http://svn.apache.org/repos/asf/tomee/tomee/trunk/server/openejb-cxf-transport/src/main/java/org/apache/openejb/server/cxf/transport/util/CxfContainerClassLoader.java
> > > )
> > > and the webapp loader are different. I'm 100% sure that's a regression
> > > even if build passes and we can't keep it.
> > >
> > > This makes equals not symmetric so using JVM equals/hashCode is the
> > > best we can do.
> > >
> > > If you have time to add tests it would be great, the issue can come
> > > from client/server and jaxws/jaxrs. It can cause linkagerror,
> > > classcast or even prevent deployment.
> > > Romain Manni-Bucau
> > > Twitter: @rmannibucau
> > > Blog: http://rmannibucau.wordpress.com/
> > > LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > > Github: https://github.com/rmannibucau
> > >
> > >
> > >
> > > 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > > It didn't locally. It takes ~2 hrs to have an exception or not.
> > > > It should happen in the next 1:15 hrs. If anything happens we can
> > always
> > > > revert it back. :)
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > On Tue, Dec 3, 2013 at 4:19 PM, Romain Manni-Bucau <
> > > rmannibucau@gmail.com>wrote:
> > > >
> > > >> it does for sure, not sure we have tests btw
> > > >> Romain Manni-Bucau
> > > >> Twitter: @rmannibucau
> > > >> Blog: http://rmannibucau.wordpress.com/
> > > >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > > >> Github: https://github.com/rmannibucau
> > > >>
> > > >>
> > > >>
> > > >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > >> > I tested it locally without the methods.
> > > >> > Committed the code to see if the build server complains.
> > > >> >
> > > >> > []s,
> > > >> > Thiago.
> > > >> >
> > > >> >
> > > >> >
> > > >> > On Tue, Dec 3, 2013 at 1:11 PM, Romain Manni-Bucau <
> > > >> rmannibucau@gmail.com>wrote:
> > > >> >
> > > >> >> hashcode is just a cached impl, equals is mandatory because of
> the
> > > >> >> hack we have to isolate a bit cxf from apps.
> > > >> >>
> > > >> >> If you fix isolation you can remove it ;).
> > > >> >> Romain Manni-Bucau
> > > >> >> Twitter: @rmannibucau
> > > >> >> Blog: http://rmannibucau.wordpress.com/
> > > >> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > > >> >> Github: https://github.com/rmannibucau
> > > >> >>
> > > >> >>
> > > >> >>
> > > >> >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > >> >> > The equals and hashCode look weird.
> > > >> >> >
> > > >> >> > ****************************************
> > > >> >> > @Override
> > > >> >> > public boolean equals(final Object other) {
> > > >> >> > return other != null &&
> > > ClassLoader.class.isInstance(other) &&
> > > >> >> > hashCode() == other.hashCode();
> > > >> >> > }
> > > >> >> >
> > > >> >> > @Override
> > > >> >> > public int hashCode() {
> > > >> >> > return hashCode;
> > > >> >> > }
> > > >> >> > ****************************************
> > > >> >> >
> > > >> >> > I wonder if we need these methods at all. Usually equal
> hashCodes
> > > does
> > > >> >> not
> > > >> >> > mean that "equals" is true, right?
> > > >> >> >
> > > >> >> > []s,
> > > >> >> > Thiago.
> > > >> >> >
> > > >> >> >
> > > >> >> >
> > > >> >> >
> > > >> >> > On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org>
> wrote:
> > > >> >> >
> > > >> >> >> Author: rmannibucau
> > > >> >> >> Date: Tue Dec 3 17:57:19 2013
> > > >> >> >> New Revision: 1547499
> > > >> >> >>
> > > >> >> >> URL: http://svn.apache.org/r1547499
> > > >> >> >> Log:
> > > >> >> >> correct hashcode in lazywebappclassloader
> > > >> >> >>
> > > >> >> >> Modified:
> > > >> >> >>
> > > >> >> >>
> > > >> >>
> > > >>
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > >> >> >>
> > > >> >> >> Modified:
> > > >> >> >>
> > > >> >>
> > > >>
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > >> >> >> URL:
> > > >> >> >>
> > > >> >>
> > > >>
> > >
> >
> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
> > > >> >> >>
> > > >> >> >>
> > > >> >>
> > > >>
> > >
> >
> ==============================================================================
> > > >> >> >> ---
> > > >> >> >>
> > > >> >>
> > > >>
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > >> >> >> (original)
> > > >> >> >> +++
> > > >> >> >>
> > > >> >>
> > > >>
> > >
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > > >> >> >> Tue Dec 3 17:57:19 2013
> > > >> >> >> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
> > > >> >> >> private boolean restarting = false;
> > > >> >> >> private boolean forceStopPhase =
> > > >> >> >>
> > > >> >>
> > > >>
> > >
> >
> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
> > > >> >> >> "false"));
> > > >> >> >> private ClassLoaderConfigurer configurer = null;
> > > >> >> >> + private final int hashCode;
> > > >> >> >>
> > > >> >> >> public LazyStopWebappClassLoader() {
> > > >> >> >> - construct();
> > > >> >> >> + hashCode = construct();
> > > >> >> >> }
> > > >> >> >>
> > > >> >> >> public LazyStopWebappClassLoader(final ClassLoader
> parent)
> > {
> > > >> >> >> super(parent);
> > > >> >> >> - construct();
> > > >> >> >> + hashCode = construct();
> > > >> >> >> }
> > > >> >> >>
> > > >> >> >> - private void construct() {
> > > >> >> >> + private int construct() {
> > > >> >> >> setDelegate(isDelegate());
> > > >> >> >> configurer = INIT_CONFIGURER.get();
> > > >> >> >> + return super.hashCode();
> > > >> >> >> }
> > > >> >> >>
> > > >> >> >> @Override
> > > >> >> >> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
> > > >> >> >> }
> > > >> >> >>
> > > >> >> >> @Override
> > > >> >> >> - public int hashCode() { // could be improved a bit adding
> > the
> > > >> host
> > > >> >> >> and ensuring contextName != null, an alternative is getURLs()
> > but
> > > it
> > > >> is
> > > >> >> >> longer
> > > >> >> >> - final String name = getContextName();
> > > >> >> >> - if (name != null) {
> > > >> >> >> - return name.hashCode();
> > > >> >> >> - }
> > > >> >> >> - return super.hashCode();
> > > >> >> >> + public int hashCode() {
> > > >> >> >> + return hashCode;
> > > >> >> >> }
> > > >> >> >>
> > > >> >> >> @Override
> > > >> >> >>
> > > >> >> >>
> > > >> >> >>
> > > >> >>
> > > >>
> > >
> >
>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
The more obvious issue comes from the fact we and libs like deltaspike use
classloader as marker so if both are not equals when needed it doesnt work
(we dont find cdi context ror instance)
Le 3 déc. 2013 23:34, "Thiago Veronezi" <th...@veronezi.org> a écrit :
> >>If you have time to add tests it would be great
> I will test it a little bit further.
>
> But I didn't quite get the problem yet... sorry! :)
>
>
> This is our implementation (without the cache)...
>
> *****************************************************************************************************
> public boolean equals(final Object other) {
> return other != null && ClassLoader.class.isInstance(other) &&
> hashCode() == other.hashCode();
> }
>
> public int hashCode() {
> return super.hashCode();
> }
>
> *****************************************************************************************************
>
> This is the default implementation...
>
> *****************************************************************************************************
> public boolean equals(Object obj) {
> return (this == obj);
> }
>
> public native int hashCode();
>
> *****************************************************************************************************
>
> The default method will return the same integer if "this == other"; and we
> are basically comparing the result of both hashCode() calls. Isn't it the
> same thing?
>
> []s,
> Thiago.
>
>
>
>
>
> On Tue, Dec 3, 2013 at 5:11 PM, Romain Manni-Bucau <rmannibucau@gmail.com
> >wrote:
>
> > well, the issue is without doing it the classloader used for cxf
> > (
> >
> http://svn.apache.org/repos/asf/tomee/tomee/trunk/server/openejb-cxf-transport/src/main/java/org/apache/openejb/server/cxf/transport/util/CxfContainerClassLoader.java
> > )
> > and the webapp loader are different. I'm 100% sure that's a regression
> > even if build passes and we can't keep it.
> >
> > This makes equals not symmetric so using JVM equals/hashCode is the
> > best we can do.
> >
> > If you have time to add tests it would be great, the issue can come
> > from client/server and jaxws/jaxrs. It can cause linkagerror,
> > classcast or even prevent deployment.
> > Romain Manni-Bucau
> > Twitter: @rmannibucau
> > Blog: http://rmannibucau.wordpress.com/
> > LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > Github: https://github.com/rmannibucau
> >
> >
> >
> > 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > > It didn't locally. It takes ~2 hrs to have an exception or not.
> > > It should happen in the next 1:15 hrs. If anything happens we can
> always
> > > revert it back. :)
> > >
> > >
> > >
> > >
> > >
> > > On Tue, Dec 3, 2013 at 4:19 PM, Romain Manni-Bucau <
> > rmannibucau@gmail.com>wrote:
> > >
> > >> it does for sure, not sure we have tests btw
> > >> Romain Manni-Bucau
> > >> Twitter: @rmannibucau
> > >> Blog: http://rmannibucau.wordpress.com/
> > >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > >> Github: https://github.com/rmannibucau
> > >>
> > >>
> > >>
> > >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > >> > I tested it locally without the methods.
> > >> > Committed the code to see if the build server complains.
> > >> >
> > >> > []s,
> > >> > Thiago.
> > >> >
> > >> >
> > >> >
> > >> > On Tue, Dec 3, 2013 at 1:11 PM, Romain Manni-Bucau <
> > >> rmannibucau@gmail.com>wrote:
> > >> >
> > >> >> hashcode is just a cached impl, equals is mandatory because of the
> > >> >> hack we have to isolate a bit cxf from apps.
> > >> >>
> > >> >> If you fix isolation you can remove it ;).
> > >> >> Romain Manni-Bucau
> > >> >> Twitter: @rmannibucau
> > >> >> Blog: http://rmannibucau.wordpress.com/
> > >> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> > >> >> Github: https://github.com/rmannibucau
> > >> >>
> > >> >>
> > >> >>
> > >> >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > >> >> > The equals and hashCode look weird.
> > >> >> >
> > >> >> > ****************************************
> > >> >> > @Override
> > >> >> > public boolean equals(final Object other) {
> > >> >> > return other != null &&
> > ClassLoader.class.isInstance(other) &&
> > >> >> > hashCode() == other.hashCode();
> > >> >> > }
> > >> >> >
> > >> >> > @Override
> > >> >> > public int hashCode() {
> > >> >> > return hashCode;
> > >> >> > }
> > >> >> > ****************************************
> > >> >> >
> > >> >> > I wonder if we need these methods at all. Usually equal hashCodes
> > does
> > >> >> not
> > >> >> > mean that "equals" is true, right?
> > >> >> >
> > >> >> > []s,
> > >> >> > Thiago.
> > >> >> >
> > >> >> >
> > >> >> >
> > >> >> >
> > >> >> > On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org> wrote:
> > >> >> >
> > >> >> >> Author: rmannibucau
> > >> >> >> Date: Tue Dec 3 17:57:19 2013
> > >> >> >> New Revision: 1547499
> > >> >> >>
> > >> >> >> URL: http://svn.apache.org/r1547499
> > >> >> >> Log:
> > >> >> >> correct hashcode in lazywebappclassloader
> > >> >> >>
> > >> >> >> Modified:
> > >> >> >>
> > >> >> >>
> > >> >>
> > >>
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > >> >> >>
> > >> >> >> Modified:
> > >> >> >>
> > >> >>
> > >>
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > >> >> >> URL:
> > >> >> >>
> > >> >>
> > >>
> >
> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
> > >> >> >>
> > >> >> >>
> > >> >>
> > >>
> >
> ==============================================================================
> > >> >> >> ---
> > >> >> >>
> > >> >>
> > >>
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > >> >> >> (original)
> > >> >> >> +++
> > >> >> >>
> > >> >>
> > >>
> >
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> > >> >> >> Tue Dec 3 17:57:19 2013
> > >> >> >> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
> > >> >> >> private boolean restarting = false;
> > >> >> >> private boolean forceStopPhase =
> > >> >> >>
> > >> >>
> > >>
> >
> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
> > >> >> >> "false"));
> > >> >> >> private ClassLoaderConfigurer configurer = null;
> > >> >> >> + private final int hashCode;
> > >> >> >>
> > >> >> >> public LazyStopWebappClassLoader() {
> > >> >> >> - construct();
> > >> >> >> + hashCode = construct();
> > >> >> >> }
> > >> >> >>
> > >> >> >> public LazyStopWebappClassLoader(final ClassLoader parent)
> {
> > >> >> >> super(parent);
> > >> >> >> - construct();
> > >> >> >> + hashCode = construct();
> > >> >> >> }
> > >> >> >>
> > >> >> >> - private void construct() {
> > >> >> >> + private int construct() {
> > >> >> >> setDelegate(isDelegate());
> > >> >> >> configurer = INIT_CONFIGURER.get();
> > >> >> >> + return super.hashCode();
> > >> >> >> }
> > >> >> >>
> > >> >> >> @Override
> > >> >> >> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
> > >> >> >> }
> > >> >> >>
> > >> >> >> @Override
> > >> >> >> - public int hashCode() { // could be improved a bit adding
> the
> > >> host
> > >> >> >> and ensuring contextName != null, an alternative is getURLs()
> but
> > it
> > >> is
> > >> >> >> longer
> > >> >> >> - final String name = getContextName();
> > >> >> >> - if (name != null) {
> > >> >> >> - return name.hashCode();
> > >> >> >> - }
> > >> >> >> - return super.hashCode();
> > >> >> >> + public int hashCode() {
> > >> >> >> + return hashCode;
> > >> >> >> }
> > >> >> >>
> > >> >> >> @Override
> > >> >> >>
> > >> >> >>
> > >> >> >>
> > >> >>
> > >>
> >
>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Thiago Veronezi <th...@veronezi.org>.
>>If you have time to add tests it would be great
I will test it a little bit further.
But I didn't quite get the problem yet... sorry! :)
This is our implementation (without the cache)...
*****************************************************************************************************
public boolean equals(final Object other) {
return other != null && ClassLoader.class.isInstance(other) &&
hashCode() == other.hashCode();
}
public int hashCode() {
return super.hashCode();
}
*****************************************************************************************************
This is the default implementation...
*****************************************************************************************************
public boolean equals(Object obj) {
return (this == obj);
}
public native int hashCode();
*****************************************************************************************************
The default method will return the same integer if "this == other"; and we
are basically comparing the result of both hashCode() calls. Isn't it the
same thing?
[]s,
Thiago.
On Tue, Dec 3, 2013 at 5:11 PM, Romain Manni-Bucau <rm...@gmail.com>wrote:
> well, the issue is without doing it the classloader used for cxf
> (
> http://svn.apache.org/repos/asf/tomee/tomee/trunk/server/openejb-cxf-transport/src/main/java/org/apache/openejb/server/cxf/transport/util/CxfContainerClassLoader.java
> )
> and the webapp loader are different. I'm 100% sure that's a regression
> even if build passes and we can't keep it.
>
> This makes equals not symmetric so using JVM equals/hashCode is the
> best we can do.
>
> If you have time to add tests it would be great, the issue can come
> from client/server and jaxws/jaxrs. It can cause linkagerror,
> classcast or even prevent deployment.
> Romain Manni-Bucau
> Twitter: @rmannibucau
> Blog: http://rmannibucau.wordpress.com/
> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> Github: https://github.com/rmannibucau
>
>
>
> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > It didn't locally. It takes ~2 hrs to have an exception or not.
> > It should happen in the next 1:15 hrs. If anything happens we can always
> > revert it back. :)
> >
> >
> >
> >
> >
> > On Tue, Dec 3, 2013 at 4:19 PM, Romain Manni-Bucau <
> rmannibucau@gmail.com>wrote:
> >
> >> it does for sure, not sure we have tests btw
> >> Romain Manni-Bucau
> >> Twitter: @rmannibucau
> >> Blog: http://rmannibucau.wordpress.com/
> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> >> Github: https://github.com/rmannibucau
> >>
> >>
> >>
> >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> >> > I tested it locally without the methods.
> >> > Committed the code to see if the build server complains.
> >> >
> >> > []s,
> >> > Thiago.
> >> >
> >> >
> >> >
> >> > On Tue, Dec 3, 2013 at 1:11 PM, Romain Manni-Bucau <
> >> rmannibucau@gmail.com>wrote:
> >> >
> >> >> hashcode is just a cached impl, equals is mandatory because of the
> >> >> hack we have to isolate a bit cxf from apps.
> >> >>
> >> >> If you fix isolation you can remove it ;).
> >> >> Romain Manni-Bucau
> >> >> Twitter: @rmannibucau
> >> >> Blog: http://rmannibucau.wordpress.com/
> >> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> >> >> Github: https://github.com/rmannibucau
> >> >>
> >> >>
> >> >>
> >> >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> >> >> > The equals and hashCode look weird.
> >> >> >
> >> >> > ****************************************
> >> >> > @Override
> >> >> > public boolean equals(final Object other) {
> >> >> > return other != null &&
> ClassLoader.class.isInstance(other) &&
> >> >> > hashCode() == other.hashCode();
> >> >> > }
> >> >> >
> >> >> > @Override
> >> >> > public int hashCode() {
> >> >> > return hashCode;
> >> >> > }
> >> >> > ****************************************
> >> >> >
> >> >> > I wonder if we need these methods at all. Usually equal hashCodes
> does
> >> >> not
> >> >> > mean that "equals" is true, right?
> >> >> >
> >> >> > []s,
> >> >> > Thiago.
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> > On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org> wrote:
> >> >> >
> >> >> >> Author: rmannibucau
> >> >> >> Date: Tue Dec 3 17:57:19 2013
> >> >> >> New Revision: 1547499
> >> >> >>
> >> >> >> URL: http://svn.apache.org/r1547499
> >> >> >> Log:
> >> >> >> correct hashcode in lazywebappclassloader
> >> >> >>
> >> >> >> Modified:
> >> >> >>
> >> >> >>
> >> >>
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> >> >>
> >> >> >> Modified:
> >> >> >>
> >> >>
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> >> >> URL:
> >> >> >>
> >> >>
> >>
> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
> >> >> >>
> >> >> >>
> >> >>
> >>
> ==============================================================================
> >> >> >> ---
> >> >> >>
> >> >>
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> >> >> (original)
> >> >> >> +++
> >> >> >>
> >> >>
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> >> >> Tue Dec 3 17:57:19 2013
> >> >> >> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
> >> >> >> private boolean restarting = false;
> >> >> >> private boolean forceStopPhase =
> >> >> >>
> >> >>
> >>
> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
> >> >> >> "false"));
> >> >> >> private ClassLoaderConfigurer configurer = null;
> >> >> >> + private final int hashCode;
> >> >> >>
> >> >> >> public LazyStopWebappClassLoader() {
> >> >> >> - construct();
> >> >> >> + hashCode = construct();
> >> >> >> }
> >> >> >>
> >> >> >> public LazyStopWebappClassLoader(final ClassLoader parent) {
> >> >> >> super(parent);
> >> >> >> - construct();
> >> >> >> + hashCode = construct();
> >> >> >> }
> >> >> >>
> >> >> >> - private void construct() {
> >> >> >> + private int construct() {
> >> >> >> setDelegate(isDelegate());
> >> >> >> configurer = INIT_CONFIGURER.get();
> >> >> >> + return super.hashCode();
> >> >> >> }
> >> >> >>
> >> >> >> @Override
> >> >> >> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
> >> >> >> }
> >> >> >>
> >> >> >> @Override
> >> >> >> - public int hashCode() { // could be improved a bit adding the
> >> host
> >> >> >> and ensuring contextName != null, an alternative is getURLs() but
> it
> >> is
> >> >> >> longer
> >> >> >> - final String name = getContextName();
> >> >> >> - if (name != null) {
> >> >> >> - return name.hashCode();
> >> >> >> - }
> >> >> >> - return super.hashCode();
> >> >> >> + public int hashCode() {
> >> >> >> + return hashCode;
> >> >> >> }
> >> >> >>
> >> >> >> @Override
> >> >> >>
> >> >> >>
> >> >> >>
> >> >>
> >>
>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
well, the issue is without doing it the classloader used for cxf
(http://svn.apache.org/repos/asf/tomee/tomee/trunk/server/openejb-cxf-transport/src/main/java/org/apache/openejb/server/cxf/transport/util/CxfContainerClassLoader.java)
and the webapp loader are different. I'm 100% sure that's a regression
even if build passes and we can't keep it.
This makes equals not symmetric so using JVM equals/hashCode is the
best we can do.
If you have time to add tests it would be great, the issue can come
from client/server and jaxws/jaxrs. It can cause linkagerror,
classcast or even prevent deployment.
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> It didn't locally. It takes ~2 hrs to have an exception or not.
> It should happen in the next 1:15 hrs. If anything happens we can always
> revert it back. :)
>
>
>
>
>
> On Tue, Dec 3, 2013 at 4:19 PM, Romain Manni-Bucau <rm...@gmail.com>wrote:
>
>> it does for sure, not sure we have tests btw
>> Romain Manni-Bucau
>> Twitter: @rmannibucau
>> Blog: http://rmannibucau.wordpress.com/
>> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> Github: https://github.com/rmannibucau
>>
>>
>>
>> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
>> > I tested it locally without the methods.
>> > Committed the code to see if the build server complains.
>> >
>> > []s,
>> > Thiago.
>> >
>> >
>> >
>> > On Tue, Dec 3, 2013 at 1:11 PM, Romain Manni-Bucau <
>> rmannibucau@gmail.com>wrote:
>> >
>> >> hashcode is just a cached impl, equals is mandatory because of the
>> >> hack we have to isolate a bit cxf from apps.
>> >>
>> >> If you fix isolation you can remove it ;).
>> >> Romain Manni-Bucau
>> >> Twitter: @rmannibucau
>> >> Blog: http://rmannibucau.wordpress.com/
>> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> >> Github: https://github.com/rmannibucau
>> >>
>> >>
>> >>
>> >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
>> >> > The equals and hashCode look weird.
>> >> >
>> >> > ****************************************
>> >> > @Override
>> >> > public boolean equals(final Object other) {
>> >> > return other != null && ClassLoader.class.isInstance(other) &&
>> >> > hashCode() == other.hashCode();
>> >> > }
>> >> >
>> >> > @Override
>> >> > public int hashCode() {
>> >> > return hashCode;
>> >> > }
>> >> > ****************************************
>> >> >
>> >> > I wonder if we need these methods at all. Usually equal hashCodes does
>> >> not
>> >> > mean that "equals" is true, right?
>> >> >
>> >> > []s,
>> >> > Thiago.
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org> wrote:
>> >> >
>> >> >> Author: rmannibucau
>> >> >> Date: Tue Dec 3 17:57:19 2013
>> >> >> New Revision: 1547499
>> >> >>
>> >> >> URL: http://svn.apache.org/r1547499
>> >> >> Log:
>> >> >> correct hashcode in lazywebappclassloader
>> >> >>
>> >> >> Modified:
>> >> >>
>> >> >>
>> >>
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> >> >>
>> >> >> Modified:
>> >> >>
>> >>
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> >> >> URL:
>> >> >>
>> >>
>> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
>> >> >>
>> >> >>
>> >>
>> ==============================================================================
>> >> >> ---
>> >> >>
>> >>
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> >> >> (original)
>> >> >> +++
>> >> >>
>> >>
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> >> >> Tue Dec 3 17:57:19 2013
>> >> >> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
>> >> >> private boolean restarting = false;
>> >> >> private boolean forceStopPhase =
>> >> >>
>> >>
>> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
>> >> >> "false"));
>> >> >> private ClassLoaderConfigurer configurer = null;
>> >> >> + private final int hashCode;
>> >> >>
>> >> >> public LazyStopWebappClassLoader() {
>> >> >> - construct();
>> >> >> + hashCode = construct();
>> >> >> }
>> >> >>
>> >> >> public LazyStopWebappClassLoader(final ClassLoader parent) {
>> >> >> super(parent);
>> >> >> - construct();
>> >> >> + hashCode = construct();
>> >> >> }
>> >> >>
>> >> >> - private void construct() {
>> >> >> + private int construct() {
>> >> >> setDelegate(isDelegate());
>> >> >> configurer = INIT_CONFIGURER.get();
>> >> >> + return super.hashCode();
>> >> >> }
>> >> >>
>> >> >> @Override
>> >> >> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
>> >> >> }
>> >> >>
>> >> >> @Override
>> >> >> - public int hashCode() { // could be improved a bit adding the
>> host
>> >> >> and ensuring contextName != null, an alternative is getURLs() but it
>> is
>> >> >> longer
>> >> >> - final String name = getContextName();
>> >> >> - if (name != null) {
>> >> >> - return name.hashCode();
>> >> >> - }
>> >> >> - return super.hashCode();
>> >> >> + public int hashCode() {
>> >> >> + return hashCode;
>> >> >> }
>> >> >>
>> >> >> @Override
>> >> >>
>> >> >>
>> >> >>
>> >>
>>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Thiago Veronezi <th...@veronezi.org>.
It didn't locally. It takes ~2 hrs to have an exception or not.
It should happen in the next 1:15 hrs. If anything happens we can always
revert it back. :)
On Tue, Dec 3, 2013 at 4:19 PM, Romain Manni-Bucau <rm...@gmail.com>wrote:
> it does for sure, not sure we have tests btw
> Romain Manni-Bucau
> Twitter: @rmannibucau
> Blog: http://rmannibucau.wordpress.com/
> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> Github: https://github.com/rmannibucau
>
>
>
> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > I tested it locally without the methods.
> > Committed the code to see if the build server complains.
> >
> > []s,
> > Thiago.
> >
> >
> >
> > On Tue, Dec 3, 2013 at 1:11 PM, Romain Manni-Bucau <
> rmannibucau@gmail.com>wrote:
> >
> >> hashcode is just a cached impl, equals is mandatory because of the
> >> hack we have to isolate a bit cxf from apps.
> >>
> >> If you fix isolation you can remove it ;).
> >> Romain Manni-Bucau
> >> Twitter: @rmannibucau
> >> Blog: http://rmannibucau.wordpress.com/
> >> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> >> Github: https://github.com/rmannibucau
> >>
> >>
> >>
> >> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> >> > The equals and hashCode look weird.
> >> >
> >> > ****************************************
> >> > @Override
> >> > public boolean equals(final Object other) {
> >> > return other != null && ClassLoader.class.isInstance(other) &&
> >> > hashCode() == other.hashCode();
> >> > }
> >> >
> >> > @Override
> >> > public int hashCode() {
> >> > return hashCode;
> >> > }
> >> > ****************************************
> >> >
> >> > I wonder if we need these methods at all. Usually equal hashCodes does
> >> not
> >> > mean that "equals" is true, right?
> >> >
> >> > []s,
> >> > Thiago.
> >> >
> >> >
> >> >
> >> >
> >> > On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org> wrote:
> >> >
> >> >> Author: rmannibucau
> >> >> Date: Tue Dec 3 17:57:19 2013
> >> >> New Revision: 1547499
> >> >>
> >> >> URL: http://svn.apache.org/r1547499
> >> >> Log:
> >> >> correct hashcode in lazywebappclassloader
> >> >>
> >> >> Modified:
> >> >>
> >> >>
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> >>
> >> >> Modified:
> >> >>
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> >> URL:
> >> >>
> >>
> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
> >> >>
> >> >>
> >>
> ==============================================================================
> >> >> ---
> >> >>
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> >> (original)
> >> >> +++
> >> >>
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> >> Tue Dec 3 17:57:19 2013
> >> >> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
> >> >> private boolean restarting = false;
> >> >> private boolean forceStopPhase =
> >> >>
> >>
> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
> >> >> "false"));
> >> >> private ClassLoaderConfigurer configurer = null;
> >> >> + private final int hashCode;
> >> >>
> >> >> public LazyStopWebappClassLoader() {
> >> >> - construct();
> >> >> + hashCode = construct();
> >> >> }
> >> >>
> >> >> public LazyStopWebappClassLoader(final ClassLoader parent) {
> >> >> super(parent);
> >> >> - construct();
> >> >> + hashCode = construct();
> >> >> }
> >> >>
> >> >> - private void construct() {
> >> >> + private int construct() {
> >> >> setDelegate(isDelegate());
> >> >> configurer = INIT_CONFIGURER.get();
> >> >> + return super.hashCode();
> >> >> }
> >> >>
> >> >> @Override
> >> >> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
> >> >> }
> >> >>
> >> >> @Override
> >> >> - public int hashCode() { // could be improved a bit adding the
> host
> >> >> and ensuring contextName != null, an alternative is getURLs() but it
> is
> >> >> longer
> >> >> - final String name = getContextName();
> >> >> - if (name != null) {
> >> >> - return name.hashCode();
> >> >> - }
> >> >> - return super.hashCode();
> >> >> + public int hashCode() {
> >> >> + return hashCode;
> >> >> }
> >> >>
> >> >> @Override
> >> >>
> >> >>
> >> >>
> >>
>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
it does for sure, not sure we have tests btw
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> I tested it locally without the methods.
> Committed the code to see if the build server complains.
>
> []s,
> Thiago.
>
>
>
> On Tue, Dec 3, 2013 at 1:11 PM, Romain Manni-Bucau <rm...@gmail.com>wrote:
>
>> hashcode is just a cached impl, equals is mandatory because of the
>> hack we have to isolate a bit cxf from apps.
>>
>> If you fix isolation you can remove it ;).
>> Romain Manni-Bucau
>> Twitter: @rmannibucau
>> Blog: http://rmannibucau.wordpress.com/
>> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> Github: https://github.com/rmannibucau
>>
>>
>>
>> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
>> > The equals and hashCode look weird.
>> >
>> > ****************************************
>> > @Override
>> > public boolean equals(final Object other) {
>> > return other != null && ClassLoader.class.isInstance(other) &&
>> > hashCode() == other.hashCode();
>> > }
>> >
>> > @Override
>> > public int hashCode() {
>> > return hashCode;
>> > }
>> > ****************************************
>> >
>> > I wonder if we need these methods at all. Usually equal hashCodes does
>> not
>> > mean that "equals" is true, right?
>> >
>> > []s,
>> > Thiago.
>> >
>> >
>> >
>> >
>> > On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org> wrote:
>> >
>> >> Author: rmannibucau
>> >> Date: Tue Dec 3 17:57:19 2013
>> >> New Revision: 1547499
>> >>
>> >> URL: http://svn.apache.org/r1547499
>> >> Log:
>> >> correct hashcode in lazywebappclassloader
>> >>
>> >> Modified:
>> >>
>> >>
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> >>
>> >> Modified:
>> >>
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> >> URL:
>> >>
>> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
>> >>
>> >>
>> ==============================================================================
>> >> ---
>> >>
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> >> (original)
>> >> +++
>> >>
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> >> Tue Dec 3 17:57:19 2013
>> >> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
>> >> private boolean restarting = false;
>> >> private boolean forceStopPhase =
>> >>
>> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
>> >> "false"));
>> >> private ClassLoaderConfigurer configurer = null;
>> >> + private final int hashCode;
>> >>
>> >> public LazyStopWebappClassLoader() {
>> >> - construct();
>> >> + hashCode = construct();
>> >> }
>> >>
>> >> public LazyStopWebappClassLoader(final ClassLoader parent) {
>> >> super(parent);
>> >> - construct();
>> >> + hashCode = construct();
>> >> }
>> >>
>> >> - private void construct() {
>> >> + private int construct() {
>> >> setDelegate(isDelegate());
>> >> configurer = INIT_CONFIGURER.get();
>> >> + return super.hashCode();
>> >> }
>> >>
>> >> @Override
>> >> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
>> >> }
>> >>
>> >> @Override
>> >> - public int hashCode() { // could be improved a bit adding the host
>> >> and ensuring contextName != null, an alternative is getURLs() but it is
>> >> longer
>> >> - final String name = getContextName();
>> >> - if (name != null) {
>> >> - return name.hashCode();
>> >> - }
>> >> - return super.hashCode();
>> >> + public int hashCode() {
>> >> + return hashCode;
>> >> }
>> >>
>> >> @Override
>> >>
>> >>
>> >>
>>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Thiago Veronezi <th...@veronezi.org>.
I tested it locally without the methods.
Committed the code to see if the build server complains.
[]s,
Thiago.
On Tue, Dec 3, 2013 at 1:11 PM, Romain Manni-Bucau <rm...@gmail.com>wrote:
> hashcode is just a cached impl, equals is mandatory because of the
> hack we have to isolate a bit cxf from apps.
>
> If you fix isolation you can remove it ;).
> Romain Manni-Bucau
> Twitter: @rmannibucau
> Blog: http://rmannibucau.wordpress.com/
> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> Github: https://github.com/rmannibucau
>
>
>
> 2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> > The equals and hashCode look weird.
> >
> > ****************************************
> > @Override
> > public boolean equals(final Object other) {
> > return other != null && ClassLoader.class.isInstance(other) &&
> > hashCode() == other.hashCode();
> > }
> >
> > @Override
> > public int hashCode() {
> > return hashCode;
> > }
> > ****************************************
> >
> > I wonder if we need these methods at all. Usually equal hashCodes does
> not
> > mean that "equals" is true, right?
> >
> > []s,
> > Thiago.
> >
> >
> >
> >
> > On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org> wrote:
> >
> >> Author: rmannibucau
> >> Date: Tue Dec 3 17:57:19 2013
> >> New Revision: 1547499
> >>
> >> URL: http://svn.apache.org/r1547499
> >> Log:
> >> correct hashcode in lazywebappclassloader
> >>
> >> Modified:
> >>
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >>
> >> Modified:
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> (original)
> >> +++
> >>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> >> Tue Dec 3 17:57:19 2013
> >> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
> >> private boolean restarting = false;
> >> private boolean forceStopPhase =
> >>
> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
> >> "false"));
> >> private ClassLoaderConfigurer configurer = null;
> >> + private final int hashCode;
> >>
> >> public LazyStopWebappClassLoader() {
> >> - construct();
> >> + hashCode = construct();
> >> }
> >>
> >> public LazyStopWebappClassLoader(final ClassLoader parent) {
> >> super(parent);
> >> - construct();
> >> + hashCode = construct();
> >> }
> >>
> >> - private void construct() {
> >> + private int construct() {
> >> setDelegate(isDelegate());
> >> configurer = INIT_CONFIGURER.get();
> >> + return super.hashCode();
> >> }
> >>
> >> @Override
> >> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
> >> }
> >>
> >> @Override
> >> - public int hashCode() { // could be improved a bit adding the host
> >> and ensuring contextName != null, an alternative is getURLs() but it is
> >> longer
> >> - final String name = getContextName();
> >> - if (name != null) {
> >> - return name.hashCode();
> >> - }
> >> - return super.hashCode();
> >> + public int hashCode() {
> >> + return hashCode;
> >> }
> >>
> >> @Override
> >>
> >>
> >>
>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
hashcode is just a cached impl, equals is mandatory because of the
hack we have to isolate a bit cxf from apps.
If you fix isolation you can remove it ;).
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2013/12/3 Thiago Veronezi <th...@veronezi.org>:
> The equals and hashCode look weird.
>
> ****************************************
> @Override
> public boolean equals(final Object other) {
> return other != null && ClassLoader.class.isInstance(other) &&
> hashCode() == other.hashCode();
> }
>
> @Override
> public int hashCode() {
> return hashCode;
> }
> ****************************************
>
> I wonder if we need these methods at all. Usually equal hashCodes does not
> mean that "equals" is true, right?
>
> []s,
> Thiago.
>
>
>
>
> On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org> wrote:
>
>> Author: rmannibucau
>> Date: Tue Dec 3 17:57:19 2013
>> New Revision: 1547499
>>
>> URL: http://svn.apache.org/r1547499
>> Log:
>> correct hashcode in lazywebappclassloader
>>
>> Modified:
>>
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>>
>> Modified:
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> URL:
>> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
>>
>> ==============================================================================
>> ---
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> (original)
>> +++
>> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>> Tue Dec 3 17:57:19 2013
>> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
>> private boolean restarting = false;
>> private boolean forceStopPhase =
>> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
>> "false"));
>> private ClassLoaderConfigurer configurer = null;
>> + private final int hashCode;
>>
>> public LazyStopWebappClassLoader() {
>> - construct();
>> + hashCode = construct();
>> }
>>
>> public LazyStopWebappClassLoader(final ClassLoader parent) {
>> super(parent);
>> - construct();
>> + hashCode = construct();
>> }
>>
>> - private void construct() {
>> + private int construct() {
>> setDelegate(isDelegate());
>> configurer = INIT_CONFIGURER.get();
>> + return super.hashCode();
>> }
>>
>> @Override
>> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
>> }
>>
>> @Override
>> - public int hashCode() { // could be improved a bit adding the host
>> and ensuring contextName != null, an alternative is getURLs() but it is
>> longer
>> - final String name = getContextName();
>> - if (name != null) {
>> - return name.hashCode();
>> - }
>> - return super.hashCode();
>> + public int hashCode() {
>> + return hashCode;
>> }
>>
>> @Override
>>
>>
>>
Re: svn commit: r1547499 - /tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
Posted by Thiago Veronezi <th...@veronezi.org>.
The equals and hashCode look weird.
****************************************
@Override
public boolean equals(final Object other) {
return other != null && ClassLoader.class.isInstance(other) &&
hashCode() == other.hashCode();
}
@Override
public int hashCode() {
return hashCode;
}
****************************************
I wonder if we need these methods at all. Usually equal hashCodes does not
mean that "equals" is true, right?
[]s,
Thiago.
On Tue, Dec 3, 2013 at 12:57 PM, <rm...@apache.org> wrote:
> Author: rmannibucau
> Date: Tue Dec 3 17:57:19 2013
> New Revision: 1547499
>
> URL: http://svn.apache.org/r1547499
> Log:
> correct hashcode in lazywebappclassloader
>
> Modified:
>
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
>
> Modified:
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> URL:
> http://svn.apache.org/viewvc/tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java?rev=1547499&r1=1547498&r2=1547499&view=diff
>
> ==============================================================================
> ---
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> (original)
> +++
> tomee/tomee/trunk/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/LazyStopWebappClassLoader.java
> Tue Dec 3 17:57:19 2013
> @@ -41,19 +41,21 @@ public class LazyStopWebappClassLoader e
> private boolean restarting = false;
> private boolean forceStopPhase =
> Boolean.parseBoolean(SystemInstance.get().getProperty("tomee.webappclassloader.force-stop-phase",
> "false"));
> private ClassLoaderConfigurer configurer = null;
> + private final int hashCode;
>
> public LazyStopWebappClassLoader() {
> - construct();
> + hashCode = construct();
> }
>
> public LazyStopWebappClassLoader(final ClassLoader parent) {
> super(parent);
> - construct();
> + hashCode = construct();
> }
>
> - private void construct() {
> + private int construct() {
> setDelegate(isDelegate());
> configurer = INIT_CONFIGURER.get();
> + return super.hashCode();
> }
>
> @Override
> @@ -211,12 +213,8 @@ public class LazyStopWebappClassLoader e
> }
>
> @Override
> - public int hashCode() { // could be improved a bit adding the host
> and ensuring contextName != null, an alternative is getURLs() but it is
> longer
> - final String name = getContextName();
> - if (name != null) {
> - return name.hashCode();
> - }
> - return super.hashCode();
> + public int hashCode() {
> + return hashCode;
> }
>
> @Override
>
>
>