[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 Hey @jeanouii, I've push a update for it Just need the review. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user jeanouii commented on the issue: https://github.com/apache/tomee/pull/176 @danielsoro and all Where are we at? Is it ready to go? ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user jgallimore commented on the issue: https://github.com/apache/tomee/pull/176 I think adding the test anyway is a good call. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 btw, it is what we already doing on tomee.sh: `"$_RUNJAVA" $DEBUG $LOGGING_MANAGER -Dopenejb.base="$CATALINA_BASE" -cp "$CLASSPATH" org.apache.openejb.cli.Bootstrap "$@"` https://github.com/apache/tomee/blob/master/tomee/apache-tomee/src/main/resources/tomee.sh#L133 ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 Well.. it is calling inside a try-statement, I believe which the close will be called in the end of the program or when some exception occur: https://github.com/apache/tomee/pull/176/files#diff-78e3cde3f4b438e49a55568e9cee40d9R162 About java -cp.. I'm going to try provide a test for it. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user rmannibucau commented on the issue: https://github.com/apache/tomee/pull/176 Thnking to users having a java -cp command launching Cli (we have a few). Side note: i think you still close the classloader before it is finished to be used - after cli execution pby ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 @rmannibucau I get it, but we isolate it just for the CLI. What is the scenario which you are seeing which can break something with that change? It should not have any impact on the application deployed in TomEE. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user rmannibucau commented on the issue: https://github.com/apache/tomee/pull/176 @danielsoro if you change the classloader it can impact the users relying on that (it can be the case if they tune java or use system classloader only libs. This is rare but already saw it. This is why i think it can be worth not changing the default behavior. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 @rmannibucau, Yeah, I'm closing it before, going to fix it. I didn't get your question about j8's user, but it should not have an impact on j8's user. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user rmannibucau commented on the issue: https://github.com/apache/tomee/pull/176 Hmm, you close the classloader before it is used so it is no more usable normally - or it will leak. Side question: any issue having an impl os ClassPath using the classloader to keep existing one for java 8 users? ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 Ok @rmannibucau, sent the correct patch, let me know what do you think. Thank you! ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 Ohh.. sorry I sent the wrong patch. Pushing the correct one. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user rmannibucau commented on the issue: https://github.com/apache/tomee/pull/176 Hope i didnt misread the diff (starts to be late ;)) but it misses a close and the urlclassloader cast will not work on all java versions right? ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 Hi @rmannibucau, patch updated. It seems to look better now. Please, review it and thank you for all the help, I really appreciate it. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 @rmannibucau, Nice.. working on it. Thank you! :) ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user rmannibucau commented on the issue: https://github.com/apache/tomee/pull/176 Yes, this is what i had in mind as well. That said if it is just for our CLI your hack is fine, we should just lock it for other cases like openejb-standalone ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 @rmannibucau, TomEE is not showing any exception when running with j11 (I was able to start and deploy a simple application on it really fine), that is not the issue. It just appears with tomee.sh when I'm using the CLI commands. Maybe applying that change just to cover the org.apache.openejb.cli.Bootstrap should works. WDYT? ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user rmannibucau commented on the issue: https://github.com/apache/tomee/pull/176 well some thoughts: 1. most of the time we dont need this dynamism so maybe we can just rework the distro and delete it 2. we already hack equals/hashcode in cxf container loader but it is not super robust (this is why we have a flag to disable it) so maybe not the best solution but current impl clearly breaks some apps 3. you can open java modules to get the URLClassPath, even of the system classloader in j11 so maybe we just document how to open it and replace the cast by some reflection? ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 Hey @rmannibucau, I updated it to call `close` and also now we call the ClassLoader.registerAsParallelCapable(); in a static block. hashcode and equals still using the super, the new class doesn't have any property which we can use to create a custom equals ou hashcode. If you have some idea how can I create ones, please show off. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user rmannibucau commented on the issue: https://github.com/apache/tomee/pull/176 Java 6 didnt have that method but we should close all our classloaders once yes. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 @rmannibucau hmm.. my question is: If we need to call close, why are not we doing it when using CustomizableURLClassLoader? is it means which we need to do the same for CustomizableURLClassLoader? ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user rmannibucau commented on the issue: https://github.com/apache/tomee/pull/176 @danielsoro classloader.close() must be called to not leak and enable to read files after the end of the container. ---
[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...
Github user danielsoro commented on the issue: https://github.com/apache/tomee/pull/176 @rmannibucau What do you want mean: should likely be closed? ---
