date:20181008

Re: TomEE 8 Release Preview

2018-10-08 Thread Romain Manni-Bucau

Le mar. 9 oct. 2018 01:56, Roberto Cortez  a
écrit :

> Romain, you mean here:
>
> https://github.com/radcortez/tomee/blob/426e0c14fede5ee5907e43e1c46e2fd51c904412/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/TomcatWebAppBuilder.java#L1772-L1774?
> <
> https://github.com/radcortez/tomee/blob/426e0c14fede5ee5907e43e1c46e2fd51c904412/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/TomcatWebAppBuilder.java#L1772-L1774
> ?>
>
> Well, why hope there is a single one? :) In case of EAR, shouldn’t all of
> them be started?
>

Yes but not here. But this code is fragile and relies on the fact it is
already started and this line skipped. This method is "start one webapp" so
we can just filter the one this lethod must start and skip others. Then
your fix is hurtless if there is any deployment change, you dont use
default deployment lifecycle and the codebase is more robust.



> David,
>
> After the RC release, we detected that issue with the MP binary and the
> way some implementations are accessing the context. I did sent a tentative
> fix, that fixes that issue, but Romain things that it may cause others, so
> we are trying to figure out what tests are we missing or if the code needs
> to change. You can help reviewing it as well.
>
> Cheers,
> Roberto
>
> > On 8 Oct 2018, at 21:50, David Blevins  wrote:
> >
> > I shot a note out to bval asking of there's a chance of getting a
> release this week.
> >
> > Assuming that's possible, is there anything standing in our way for
> putting up a release vote ourselves this week?
> >
> >
> > -David
> >
> >> On Oct 8, 2018, at 8:28 AM, Romain Manni-Bucau 
> wrote:
> >>
> >> one option can be to start only one webapp here instead of all and hope
> >> there is a single one or others are skipped ;)
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau  |  Blog
> >>  | Old Blog
> >>  | Github <
> https://github.com/rmannibucau> |
> >> LinkedIn  | Book
> >> <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >>
> >>
> >> Le lun. 8 oct. 2018 à 16:15, Roberto Cortez 
> a
> >> écrit :
> >>
> >>> Ok, thanks. Can you help to better test it?
> >>>
> >>> Cheers,
> >>> Roberto
> >>>
>  On 6 Oct 2018, at 08:26, Romain Manni-Bucau 
> >>> wrote:
> 
>  Le sam. 6 oct. 2018 00:30, Roberto Cortez  >
> >>> a
>  écrit :
> 
> > Would something like this work?
> >
> >
> >>>
> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
> > <
> >
> >>>
> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
> >>
> >
> > I’m not exactly sure what the problem might be with EAR. Web Modules,
> >>> seem
> > to also be deployed like this, so they suffer from the same issue.
> After
> > the JAX-RS app is started the context is removed.
> >
> 
> 
>  You set a single context for all webapps so code is quite miskeading
> and
>  error prone.
> 
> 
> > Please let me know what other situations you have in mind that may
> cause
> > issues?
> >
> > Cheers,
> > Roberto
> >
> >> On 4 Oct 2018, at 16:05, Roberto Cortez  >
> > wrote:
> >>
> >> I understand. Was just trying to give more detail into it.
> >>
> >> I’ll have a better look and try to come up with some test scenarios.
> >>
> >>> On 4 Oct 2018, at 10:47, Romain Manni-Bucau  >
> > wrote:
> >>>
> >>> Le jeu. 4 oct. 2018 à 11:42, Roberto Cortez
> >>>  > > a
> >>> écrit :
> >>>
>  Hi Romain,
> 
>  Well the exception being thrown is not the actual exception.
> 
>  This was only happening in the MP binary due to the OpenAPI
> Geronimo
>  implementation. In the DefaultLoader the ServletContext is
> injected,
> > but at
>  the time that the JAX-RS app is deployed, which is in the
>  AfterApplicationCreated event, the ServletContextHandler does not
> >>> have
> > a
>  Context anymore so it throws a IllegalStateException("Didnt find a
> >>> web
>  context for " + contextClassLoader). The caller for this is the
>  setApplication of the OpenAPIFilter when we try to inject it, so
> that
>  causes the exception we see in the logs.
> 
>  We never say this in Arquillian testing, because Arquillian waits
> for
> > the
>  server to start and then deploys the app. This means we are able
> to
> > get a
>  ServletContext from the request in ServletContextHandler, so it
> works
> > fine.
> 
>  I believe this is also related with the fix you did here:
>  https://issues.apache.org/jira/browse/TOMEE-1687 <
>  https://issues.apache.org/jira/browse/TOMEE-1687>
>

Re: TomEE 8 Release Preview

2018-10-08 Thread Roberto Cortez

Romain, you mean here:
https://github.com/radcortez/tomee/blob/426e0c14fede5ee5907e43e1c46e2fd51c904412/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/TomcatWebAppBuilder.java#L1772-L1774?
 


Well, why hope there is a single one? :) In case of EAR, shouldn’t all of them 
be started?

David,

After the RC release, we detected that issue with the MP binary and the way 
some implementations are accessing the context. I did sent a tentative fix, 
that fixes that issue, but Romain things that it may cause others, so we are 
trying to figure out what tests are we missing or if the code needs to change. 
You can help reviewing it as well.

Cheers,
Roberto

> On 8 Oct 2018, at 21:50, David Blevins  wrote:
> 
> I shot a note out to bval asking of there's a chance of getting a release 
> this week.
> 
> Assuming that's possible, is there anything standing in our way for putting 
> up a release vote ourselves this week?
> 
> 
> -David
> 
>> On Oct 8, 2018, at 8:28 AM, Romain Manni-Bucau  wrote:
>> 
>> one option can be to start only one webapp here instead of all and hope
>> there is a single one or others are skipped ;)
>> 
>> Romain Manni-Bucau
>> @rmannibucau  |  Blog
>>  | Old Blog
>>  | Github  
>> |
>> LinkedIn  | Book
>> 
>> 
>> 
>> Le lun. 8 oct. 2018 à 16:15, Roberto Cortez  a
>> écrit :
>> 
>>> Ok, thanks. Can you help to better test it?
>>> 
>>> Cheers,
>>> Roberto
>>> 
 On 6 Oct 2018, at 08:26, Romain Manni-Bucau 
>>> wrote:
 
 Le sam. 6 oct. 2018 00:30, Roberto Cortez 
>>> a
 écrit :
 
> Would something like this work?
> 
> 
>>> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
> <
> 
>>> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
>> 
> 
> I’m not exactly sure what the problem might be with EAR. Web Modules,
>>> seem
> to also be deployed like this, so they suffer from the same issue. After
> the JAX-RS app is started the context is removed.
> 
 
 
 You set a single context for all webapps so code is quite miskeading and
 error prone.
 
 
> Please let me know what other situations you have in mind that may cause
> issues?
> 
> Cheers,
> Roberto
> 
>> On 4 Oct 2018, at 16:05, Roberto Cortez 
> wrote:
>> 
>> I understand. Was just trying to give more detail into it.
>> 
>> I’ll have a better look and try to come up with some test scenarios.
>> 
>>> On 4 Oct 2018, at 10:47, Romain Manni-Bucau 
> wrote:
>>> 
>>> Le jeu. 4 oct. 2018 à 11:42, Roberto Cortez
>>>  > a
>>> écrit :
>>> 
 Hi Romain,
 
 Well the exception being thrown is not the actual exception.
 
 This was only happening in the MP binary due to the OpenAPI Geronimo
 implementation. In the DefaultLoader the ServletContext is injected,
> but at
 the time that the JAX-RS app is deployed, which is in the
 AfterApplicationCreated event, the ServletContextHandler does not
>>> have
> a
 Context anymore so it throws a IllegalStateException("Didnt find a
>>> web
 context for " + contextClassLoader). The caller for this is the
 setApplication of the OpenAPIFilter when we try to inject it, so that
 causes the exception we see in the logs.
 
 We never say this in Arquillian testing, because Arquillian waits for
> the
 server to start and then deploys the app. This means we are able to
> get a
 ServletContext from the request in ServletContextHandler, so it works
> fine.
 
 I believe this is also related with the fix you did here:
 https://issues.apache.org/jira/browse/TOMEE-1687 <
 https://issues.apache.org/jira/browse/TOMEE-1687>
 
 What do you think?
 
>>> 
>>> Can be but at the end the two issues are mentionned are not covered:
>>> 
>>> 1. a regression will silently come back
>>> 2. for ears we can leak the servlet context if we end up here (and
>>> then
> the
>>> app will wrongly behave)
>>> 
>>> My point is not to revert what you did but more to ensure it fixes the
>>> issue in our build.
>>> 
>>> 
 
 Cheers,
 Roberto
 
> On 4 Oct 2018, at 08:23, Romain Manni-Bucau 
 wrote:
> 
> @Roberto: do we have a test to reproduce and prevent future

Re: TomEE 8 Release Preview

2018-10-08 Thread David Blevins

I shot a note out to bval asking of there's a chance of getting a release this 
week.

Assuming that's possible, is there anything standing in our way for putting up 
a release vote ourselves this week?


-David

> On Oct 8, 2018, at 8:28 AM, Romain Manni-Bucau  wrote:
> 
> one option can be to start only one webapp here instead of all and hope
> there is a single one or others are skipped ;)
> 
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github  |
> LinkedIn  | Book
> 
> 
> 
> Le lun. 8 oct. 2018 à 16:15, Roberto Cortez  a
> écrit :
> 
>> Ok, thanks. Can you help to better test it?
>> 
>> Cheers,
>> Roberto
>> 
>>> On 6 Oct 2018, at 08:26, Romain Manni-Bucau 
>> wrote:
>>> 
>>> Le sam. 6 oct. 2018 00:30, Roberto Cortez 
>> a
>>> écrit :
>>> 
 Would something like this work?
 
 
>> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
 <
 
>> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
> 
 
 I’m not exactly sure what the problem might be with EAR. Web Modules,
>> seem
 to also be deployed like this, so they suffer from the same issue. After
 the JAX-RS app is started the context is removed.
 
>>> 
>>> 
>>> You set a single context for all webapps so code is quite miskeading and
>>> error prone.
>>> 
>>> 
 Please let me know what other situations you have in mind that may cause
 issues?
 
 Cheers,
 Roberto
 
> On 4 Oct 2018, at 16:05, Roberto Cortez 
 wrote:
> 
> I understand. Was just trying to give more detail into it.
> 
> I’ll have a better look and try to come up with some test scenarios.
> 
>> On 4 Oct 2018, at 10:47, Romain Manni-Bucau 
 wrote:
>> 
>> Le jeu. 4 oct. 2018 à 11:42, Roberto Cortez
>> >>> > a
>> écrit :
>> 
>>> Hi Romain,
>>> 
>>> Well the exception being thrown is not the actual exception.
>>> 
>>> This was only happening in the MP binary due to the OpenAPI Geronimo
>>> implementation. In the DefaultLoader the ServletContext is injected,
 but at
>>> the time that the JAX-RS app is deployed, which is in the
>>> AfterApplicationCreated event, the ServletContextHandler does not
>> have
 a
>>> Context anymore so it throws a IllegalStateException("Didnt find a
>> web
>>> context for " + contextClassLoader). The caller for this is the
>>> setApplication of the OpenAPIFilter when we try to inject it, so that
>>> causes the exception we see in the logs.
>>> 
>>> We never say this in Arquillian testing, because Arquillian waits for
 the
>>> server to start and then deploys the app. This means we are able to
 get a
>>> ServletContext from the request in ServletContextHandler, so it works
 fine.
>>> 
>>> I believe this is also related with the fix you did here:
>>> https://issues.apache.org/jira/browse/TOMEE-1687 <
>>> https://issues.apache.org/jira/browse/TOMEE-1687>
>>> 
>>> What do you think?
>>> 
>> 
>> Can be but at the end the two issues are mentionned are not covered:
>> 
>> 1. a regression will silently come back
>> 2. for ears we can leak the servlet context if we end up here (and
>> then
 the
>> app will wrongly behave)
>> 
>> My point is not to revert what you did but more to ensure it fixes the
>> issue in our build.
>> 
>> 
>>> 
>>> Cheers,
>>> Roberto
>>> 
 On 4 Oct 2018, at 08:23, Romain Manni-Bucau 
>>> wrote:
 
 @Roberto: do we have a test to reproduce and prevent future
 regressions?
>>> If
 your fix is right the error message is quite unexpected so it would
>> be
 better to ensure we don't break it unintentionnally (a side note is
 that
 setting a single webapp context and firing an event for all webapps
 can
 have the same pitfall so it can need to be reworked to ensure we
>> don't
>>> hit
 it for ears to not leak context between webapp which would be
>> another
>>> nasty
 bug).
 
 Romain Manni-Bucau
 @rmannibucau  |  Blog
  | Old Blog
  | Github <
>>> https://github.com/rmannibucau> |
 LinkedIn  | Book
 <
>>> 
 
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
 
 
 
 Le jeu. 4 oct. 2018 à 01:43, Roberto Cortez
 
>>> a
 écrit :

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

2018-10-08 Thread exabrial

Github user exabrial commented on a diff in the pull request:

https://github.com/apache/tomee/pull/175#discussion_r223483193
  
--- Diff: 
container/openejb-jpa-integration/src/main/java/org/apache/openejb/jpa/integration/eclipselink/OpenEJBServerPlatform.java
 ---
@@ -26,13 +27,14 @@
 import javax.transaction.Synchronization;
 import javax.transaction.TransactionManager;
 
-public class OpenEJBServerPlatform extends JMXServerPlatformBase {
+public class OpenEJBServerPlatform extends JMXServerPlatformBase 
implements JMXEnabledPlatform {
 public OpenEJBServerPlatform(final DatabaseSession newDatabaseSession) 
{
 super(newDatabaseSession);
 try {
 mBeanServer = MBeanServer.class.cast(
 
OpenEJBServerPlatform.class.getClassLoader().loadClass("org.apache.openejb.monitoring.LocalMBeanServer")
 .getMethod("get").invoke(null));
+this.prepareServerSpecificServicesMBean();
 } catch (final Exception e) {
 // no-op
--- End diff --

I agree with you; I don't understand why they split it out either, but they 
literally did it for _every other server_ so there's probably a good reason. 

On the fail vs noop, that was there before me. I suggest leaving it so we 
don't change current behavior (if JMX fails to initialize, we still alow the 
application to run. Right now JMX is failing.)

> 6e2a4f7c419 (Thiago Veronezi2015-11-23 14:38:43 -0500 36) 
.getMethod("get").invoke(null));
> 515306f7ba0 (Jonathan S. Fisher 2018-09-29 13:10:49 -0500 37) 
this.prepareServerSpecificServicesMBean();
> 6e2a4f7c419 (Thiago Veronezi2015-11-23 14:38:43 -0500 38) } 
catch (final Exception e) {
> 6e2a4f7c419 (Thiago Veronezi2015-11-23 14:38:43 -0500 39) 
// no-op
> 6e2a4f7c419 (Thiago Veronezi2015-11-23 14:38:43 -0500 40) }
> 6e2a4f7c419 (Thiago Veronezi2015-11-23 14:38:43 -0500 41) }
> 6e2a4f7c419 (Thiago Veronezi2015-11-23 14:38:43 -0500 42) 


---

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

2018-10-08 Thread rmannibucau

Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/175#discussion_r223478552
  
--- Diff: 
container/openejb-jpa-integration/src/main/java/org/apache/openejb/jpa/integration/eclipselink/OpenEJBServerPlatform.java
 ---
@@ -26,13 +27,14 @@
 import javax.transaction.Synchronization;
 import javax.transaction.TransactionManager;
 
-public class OpenEJBServerPlatform extends JMXServerPlatformBase {
+public class OpenEJBServerPlatform extends JMXServerPlatformBase 
implements JMXEnabledPlatform {
 public OpenEJBServerPlatform(final DatabaseSession newDatabaseSession) 
{
 super(newDatabaseSession);
 try {
 mBeanServer = MBeanServer.class.cast(
 
OpenEJBServerPlatform.class.getClassLoader().loadClass("org.apache.openejb.monitoring.LocalMBeanServer")
 .getMethod("get").invoke(null));
+this.prepareServerSpecificServicesMBean();
 } catch (final Exception e) {
 // no-op
--- End diff --

Fail instead of noop?


---

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

2018-10-08 Thread rmannibucau

Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/175#discussion_r223478337
  
--- Diff: 
container/openejb-jpa-integration/src/main/java/org/apache/openejb/jpa/integration/eclipselink/MBeanOpenEJBRuntimeServices.java
 ---
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.jpa.integration.eclipselink;
+
+import org.eclipse.persistence.internal.sessions.AbstractSession;
+import org.eclipse.persistence.sessions.Session;
+
+public class MBeanOpenEJBRuntimeServices extends OpenEJBRuntimeServices 
implements MBeanOpenEJBRuntimeServicesMBean {
--- End diff --

Was more thinking about dropping this and keeping only 
OpenEJBRuntimeServices


---

[GitHub] tomee issue #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on init

2018-10-08 Thread exabrial

Github user exabrial commented on the issue:

https://github.com/apache/tomee/pull/175
  
@rmannibucau If this looks good, could you merge?


---

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

2018-10-08 Thread exabrial

Github user exabrial commented on a diff in the pull request:

https://github.com/apache/tomee/pull/175#discussion_r223456172
  
--- Diff: 
container/openejb-jpa-integration/src/main/java/org/apache/openejb/jpa/integration/eclipselink/OpenEJBServerPlatform.java
 ---
@@ -63,4 +65,11 @@ protected void 
registerSynchronization_impl(AbstractSynchronizationListener list
 transaction.registerInterposedSynchronization(synchronization);
 }
 }
+
+@Override
+public void prepareServerSpecificServicesMBean() {
+if (getDatabaseSession() != null && shouldRegisterRuntimeBean) 
{
+ this.setRuntimeServicesMBean(new 
MBeanOpenEJBRuntimeServices(getDatabaseSession()));
--- End diff --

Good idea, I added a check there


---

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

2018-10-08 Thread exabrial

Github user exabrial commented on a diff in the pull request:

https://github.com/apache/tomee/pull/175#discussion_r223455236
  
--- Diff: 
container/openejb-jpa-integration/src/main/java/org/apache/openejb/jpa/integration/eclipselink/MBeanOpenEJBRuntimeServices.java
 ---
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.jpa.integration.eclipselink;
+
+import org.eclipse.persistence.internal.sessions.AbstractSession;
+import org.eclipse.persistence.sessions.Session;
+
+public class MBeanOpenEJBRuntimeServices extends OpenEJBRuntimeServices 
implements MBeanOpenEJBRuntimeServicesMBean {
--- End diff --

I certainly can use the Glassfish one here if you would like. But, to 
remain consistent with how all the other ones are implemented, I would suggest 
we don't.

![screen shot 2018-10-08 at 1 18 15 
pm](https://user-images.githubusercontent.com/1392297/46626331-a5618080-cafc-11e8-8f13-72905a7debe2.png)



---

Re: How about upgrading to Tomcat 8.5.34 for CVE & bug fixes?

2018-10-08 Thread exabrial12

Looks like the other Jonathan beat me to it :)



--
Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-f982480.html

Re: [GitHub] tomee pull request #172: [7.0.x][Tomee-2243] jmx on managed executors

2018-10-08 Thread exabrial12

Hey guys, can this PR get merged into 7.0.x, 7.1.x and 8.0.x? Thanks!



--
Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-f982480.html

Re: How about upgrading to Tomcat 8.5.34 for CVE & bug fixes?

2018-10-08 Thread exabrial12

Alex I'll create a PR for this for 7.0.x. Can you see if TomEE 7.1.x and
8.0.x-SNAPSHOT are affected as well?



--
Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-f982480.html

Re: TomEE 8 Release Preview

2018-10-08 Thread Romain Manni-Bucau

one option can be to start only one webapp here instead of all and hope
there is a single one or others are skipped ;)

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | Book



Le lun. 8 oct. 2018 à 16:15, Roberto Cortez  a
écrit :

> Ok, thanks. Can you help to better test it?
>
> Cheers,
> Roberto
>
> > On 6 Oct 2018, at 08:26, Romain Manni-Bucau 
> wrote:
> >
> > Le sam. 6 oct. 2018 00:30, Roberto Cortez 
> a
> > écrit :
> >
> >> Would something like this work?
> >>
> >>
> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
> >> <
> >>
> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
> >>>
> >>
> >> I’m not exactly sure what the problem might be with EAR. Web Modules,
> seem
> >> to also be deployed like this, so they suffer from the same issue. After
> >> the JAX-RS app is started the context is removed.
> >>
> >
> >
> > You set a single context for all webapps so code is quite miskeading and
> > error prone.
> >
> >
> >> Please let me know what other situations you have in mind that may cause
> >> issues?
> >>
> >> Cheers,
> >> Roberto
> >>
> >>> On 4 Oct 2018, at 16:05, Roberto Cortez 
> >> wrote:
> >>>
> >>> I understand. Was just trying to give more detail into it.
> >>>
> >>> I’ll have a better look and try to come up with some test scenarios.
> >>>
>  On 4 Oct 2018, at 10:47, Romain Manni-Bucau 
> >> wrote:
> 
>  Le jeu. 4 oct. 2018 à 11:42, Roberto Cortez
>  >> > a
>  écrit :
> 
> > Hi Romain,
> >
> > Well the exception being thrown is not the actual exception.
> >
> > This was only happening in the MP binary due to the OpenAPI Geronimo
> > implementation. In the DefaultLoader the ServletContext is injected,
> >> but at
> > the time that the JAX-RS app is deployed, which is in the
> > AfterApplicationCreated event, the ServletContextHandler does not
> have
> >> a
> > Context anymore so it throws a IllegalStateException("Didnt find a
> web
> > context for " + contextClassLoader). The caller for this is the
> > setApplication of the OpenAPIFilter when we try to inject it, so that
> > causes the exception we see in the logs.
> >
> > We never say this in Arquillian testing, because Arquillian waits for
> >> the
> > server to start and then deploys the app. This means we are able to
> >> get a
> > ServletContext from the request in ServletContextHandler, so it works
> >> fine.
> >
> > I believe this is also related with the fix you did here:
> > https://issues.apache.org/jira/browse/TOMEE-1687 <
> > https://issues.apache.org/jira/browse/TOMEE-1687>
> >
> > What do you think?
> >
> 
>  Can be but at the end the two issues are mentionned are not covered:
> 
>  1. a regression will silently come back
>  2. for ears we can leak the servlet context if we end up here (and
> then
> >> the
>  app will wrongly behave)
> 
>  My point is not to revert what you did but more to ensure it fixes the
>  issue in our build.
> 
> 
> >
> > Cheers,
> > Roberto
> >
> >> On 4 Oct 2018, at 08:23, Romain Manni-Bucau 
> > wrote:
> >>
> >> @Roberto: do we have a test to reproduce and prevent future
> >> regressions?
> > If
> >> your fix is right the error message is quite unexpected so it would
> be
> >> better to ensure we don't break it unintentionnally (a side note is
> >> that
> >> setting a single webapp context and firing an event for all webapps
> >> can
> >> have the same pitfall so it can need to be reworked to ensure we
> don't
> > hit
> >> it for ears to not leak context between webapp which would be
> another
> > nasty
> >> bug).
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau  |  Blog
> >>  | Old Blog
> >>  | Github <
> > https://github.com/rmannibucau> |
> >> LinkedIn  | Book
> >> <
> >
> >>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >>
> >>
> >>
> >> Le jeu. 4 oct. 2018 à 01:43, Roberto Cortez
> >> 
> > a
> >> écrit :
> >>
> >>> Hi Cesar,
> >>>
> >>> I think I’ve found the issue. Just pushed a fix and now waiting to
> >> the
> >>> build bot to check if everything is ok. I’ll let you know when it
> is
> > done.
> >>>
> >>> Cheers,
> >>> Roberto
> >>>
>  On 3 Oct 2018, at 22:50, Roberto Cortez
>  >>>
> >>> wrote:
> 
> 
>

Re: TomEE 8 Release Preview

2018-10-08 Thread Roberto Cortez

Ok, thanks. Can you help to better test it?

Cheers,
Roberto

> On 6 Oct 2018, at 08:26, Romain Manni-Bucau  wrote:
> 
> Le sam. 6 oct. 2018 00:30, Roberto Cortez  a
> écrit :
> 
>> Would something like this work?
>> 
>> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
>> <
>> https://github.com/apache/tomee/commit/ea13f63f21d7f06388e2c54d70120a5a98e4c65f
>>> 
>> 
>> I’m not exactly sure what the problem might be with EAR. Web Modules, seem
>> to also be deployed like this, so they suffer from the same issue. After
>> the JAX-RS app is started the context is removed.
>> 
> 
> 
> You set a single context for all webapps so code is quite miskeading and
> error prone.
> 
> 
>> Please let me know what other situations you have in mind that may cause
>> issues?
>> 
>> Cheers,
>> Roberto
>> 
>>> On 4 Oct 2018, at 16:05, Roberto Cortez 
>> wrote:
>>> 
>>> I understand. Was just trying to give more detail into it.
>>> 
>>> I’ll have a better look and try to come up with some test scenarios.
>>> 
 On 4 Oct 2018, at 10:47, Romain Manni-Bucau 
>> wrote:
 
 Le jeu. 4 oct. 2018 à 11:42, Roberto Cortez > > a
 écrit :
 
> Hi Romain,
> 
> Well the exception being thrown is not the actual exception.
> 
> This was only happening in the MP binary due to the OpenAPI Geronimo
> implementation. In the DefaultLoader the ServletContext is injected,
>> but at
> the time that the JAX-RS app is deployed, which is in the
> AfterApplicationCreated event, the ServletContextHandler does not have
>> a
> Context anymore so it throws a IllegalStateException("Didnt find a web
> context for " + contextClassLoader). The caller for this is the
> setApplication of the OpenAPIFilter when we try to inject it, so that
> causes the exception we see in the logs.
> 
> We never say this in Arquillian testing, because Arquillian waits for
>> the
> server to start and then deploys the app. This means we are able to
>> get a
> ServletContext from the request in ServletContextHandler, so it works
>> fine.
> 
> I believe this is also related with the fix you did here:
> https://issues.apache.org/jira/browse/TOMEE-1687 <
> https://issues.apache.org/jira/browse/TOMEE-1687>
> 
> What do you think?
> 
 
 Can be but at the end the two issues are mentionned are not covered:
 
 1. a regression will silently come back
 2. for ears we can leak the servlet context if we end up here (and then
>> the
 app will wrongly behave)
 
 My point is not to revert what you did but more to ensure it fixes the
 issue in our build.
 
 
> 
> Cheers,
> Roberto
> 
>> On 4 Oct 2018, at 08:23, Romain Manni-Bucau 
> wrote:
>> 
>> @Roberto: do we have a test to reproduce and prevent future
>> regressions?
> If
>> your fix is right the error message is quite unexpected so it would be
>> better to ensure we don't break it unintentionnally (a side note is
>> that
>> setting a single webapp context and firing an event for all webapps
>> can
>> have the same pitfall so it can need to be reworked to ensure we don't
> hit
>> it for ears to not leak context between webapp which would be another
> nasty
>> bug).
>> 
>> Romain Manni-Bucau
>> @rmannibucau  |  Blog
>>  | Old Blog
>>  | Github <
> https://github.com/rmannibucau> |
>> LinkedIn  | Book
>> <
> 
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>> 
>> 
>> 
>> Le jeu. 4 oct. 2018 à 01:43, Roberto Cortez
>> 
> a
>> écrit :
>> 
>>> Hi Cesar,
>>> 
>>> I think I’ve found the issue. Just pushed a fix and now waiting to
>> the
>>> build bot to check if everything is ok. I’ll let you know when it is
> done.
>>> 
>>> Cheers,
>>> Roberto
>>> 
 On 3 Oct 2018, at 22:50, Roberto Cortez >> 
>>> wrote:
 
 
 Not sure. Let me have a look.On Wednesday, October 3, 2018,
>> 8:56:13
>>> PM GMT+1, César Hernández Mendoza  wrote:
 
 Hi everyone,
 I have a simple REST app that is working fine with tomee.version
> 7.1.0,
>>> but tomee:run it's getting issues after updating it to
> 8.0.0-RC1-SNAPSHOT
>>> and also update the javaee-api to 8.0-SNAPSHOT.  mvn test runs fine
>> but
>>> when I try to run mvn clean install tomee:run I got a cxf error (I
> attached
>>> the full log file to this email).
 03-Oct-2018 11:54:26.368 SEVERE [main]
>>> org.apache.cxf.jaxrs.utils.InjectionUtils.reportServerError Method
>>> setApplication can not be accessed due to security manager
>>> restrictions03-Oct-2018

Re: TomEE 8 Release Preview

Re: TomEE 8 Release Preview

Re: TomEE 8 Release Preview

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

[GitHub] tomee issue #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on init

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

Re: How about upgrading to Tomcat 8.5.34 for CVE & bug fixes?

Re: [GitHub] tomee pull request #172: [7.0.x][Tomee-2243] jmx on managed executors

Re: How about upgrading to Tomcat 8.5.34 for CVE & bug fixes?

Re: TomEE 8 Release Preview

Re: TomEE 8 Release Preview

14 matches

Site Navigation

Mail list logo

Footer information