Thanks for merging, Jonathan.. :-) On Thu, Aug 6, 2015 at 3:57 PM, Jonathan Gallimore <[email protected] > wrote:
> Merged - thanks for the patch! I've queued a build on buildbot - the next > build here: https://ci.apache.org/builders/tomee-1.7.x-ubuntu should > include your change. > > Cheers > > Jon > > On Thu, Aug 6, 2015 at 9:57 AM, Kasun Gajasinghe <[email protected]> > wrote: > > > I have sent the pull request. Please review, and let me know if you need > to > > see any changes. > > > > https://github.com/apache/tomee/pull/18 > > > > Thanks.. > > > > On Thu, Aug 6, 2015 at 2:13 PM, Kasun Gajasinghe <[email protected]> > > wrote: > > > > > > > > That's perfect, Romain. I created the jira ticket [1]. I will send the > > > pull request as well. > > > > > > We've gone through Karaf. But it is not a possibility for us right now. > > > > > > [1] https://issues.apache.org/jira/browse/TOMEE-1622 > > > > > > Thanks. > > > > > > > > > On Thu, Aug 6, 2015 at 4:36 AM, Romain Manni-Bucau < > > [email protected]> > > > wrote: > > > > > >> I see, not sure it is good but explains better the issue. > > >> > > >> Justifies a fix on our side. > > >> > > >> Ps: did you get in touch with karaf community, they have a similar > > effort > > >> in progress > > >> On Wed, Aug 5, 2015 at 4:46 PM, Romain Manni-Bucau < > > [email protected] > > >> > > > >> wrote: > > >> > > >> > 2015-08-05 13:14 GMT+02:00 Kasun Gajasinghe <[email protected]>: > > >> > > > >> > > As I see, iterating over the System.getProperties() instance is > not > > >> > thread > > >> > > safe. It's quite possible that another thread would want to > interact > > >> with > > >> > > system properties. It may be a rare occurrence, but IMO, we need > to > > >> fix > > >> > it. > > >> > > > > >> > > > > >> > sure but before any of your code - ie not container one - can be > > >> executed? > > >> > this is the main point. > > >> > > > >> > > > >> Yes, it can be. We are starting up a embedded tomcat with Tomee > support > > in > > >> an OSGi runtime. > > >> > > >> > > >> > > > >> > > There are other threads running in our project, so I'm not saying > > the > > >> > other > > >> > > thread which interact with system properties is coming from TomEE. > > >> > > > > >> > > On Wed, Aug 5, 2015 at 4:01 PM, Romain Manni-Bucau < > > >> > [email protected]> > > >> > > wrote: > > >> > > > > >> > > > Asking cause we suppose we are thread safe here - and should be > - > > so > > >> > > > identifying the real issue can maybe require a totally different > > >> fix. > > >> > > > Le 5 août 2015 12:27, "Kasun Gajasinghe" <[email protected]> a > > >> écrit : > > >> > > > > > >> > > > > Hi Romain, > > >> > > > > > > >> > > > > I can try, but it isn't easy. This is pretty hard to > re-produce, > > >> but > > >> > we > > >> > > > did > > >> > > > > notice it few times. I believe this affects the TomEE > downstream > > >> > > projects > > >> > > > > like ours where our components will be > adding/removing/modifying > > >> > System > > >> > > > > properties. > > >> > > > > > > >> > > > > On Wed, Aug 5, 2015 at 3:52 PM, Romain Manni-Bucau < > > >> > > > [email protected]> > > >> > > > > wrote: > > >> > > > > > > >> > > > > > Hi > > >> > > > > > > > >> > > > > > Can you identify which thread? It shouldnt be possible. > > >> > > > > > Le 5 août 2015 12:20, "Kasun Gajasinghe" <[email protected] > > > > a > > >> > > écrit : > > >> > > > > > > > >> > > > > > > Hi Jonathan, > > >> > > > > > > > > >> > > > > > > I will raise an issue, and is happy to send a pull request > > via > > >> > > > GitHub. > > >> > > > > > It's > > >> > > > > > > good to see that TomEE accepts pull requests like that. > > >> > > > > > > > > >> > > > > > > Thanks, > > >> > > > > > > Kasun > > >> > > > > > > > > >> > > > > > > On Wed, Aug 5, 2015 at 3:45 PM, Jonathan Gallimore < > > >> > > > > > > [email protected] > > >> > > > > > > > wrote: > > >> > > > > > > > > >> > > > > > > > Hi there > > >> > > > > > > > > > >> > > > > > > > Thanks for raising this. Can you create a JIRA for this > > >> here: > > >> > > > > > > > https://issues.apache.org/jira? If you want to have a > go > > at > > >> > the > > >> > > > fix, > > >> > > > > > > just > > >> > > > > > > > let us know and submit either a patch attached to the > JIRA > > >> > issue > > >> > > > or a > > >> > > > > > > pull > > >> > > > > > > > request through GitHub. If not, I'll happily pick this > up > > >> and > > >> > > fix. > > >> > > > > > > > > > >> > > > > > > > We've had a few fixes on the 1.7.x branch now, so we > could > > >> look > > >> > > at > > >> > > > > > > rolling > > >> > > > > > > > a new release pretty soon. > > >> > > > > > > > > > >> > > > > > > > Regards > > >> > > > > > > > > > >> > > > > > > > Jon > > >> > > > > > > > > > >> > > > > > > > On Wed, Aug 5, 2015 at 11:10 AM, Kasun Gajasinghe < > > >> > > > [email protected] > > >> > > > > > > > >> > > > > > > > wrote: > > >> > > > > > > > > > >> > > > > > > > > Hi TomEE devs, > > >> > > > > > > > > > > >> > > > > > > > > We've been seeing the following exception [2] in an > > >> instance > > >> > > > where > > >> > > > > we > > >> > > > > > > > have > > >> > > > > > > > > extended TomEE 1.7.2. We have debugged the issue, and > it > > >> > seems > > >> > > to > > >> > > > > > have > > >> > > > > > > > > occurred since TomEE iterate over the > > >> System.getProperties() > > >> > > > > > hashtable > > >> > > > > > > > [1]. > > >> > > > > > > > > It's possible that other threads might be using the > > system > > >> > > > > properties > > >> > > > > > > > which > > >> > > > > > > > > could lead to ConcurrentModificationException. > > >> > > > > > > > > > > >> > > > > > > > > Best way fix thing would to create a clone of the > system > > >> > > > > properties, > > >> > > > > > > and > > >> > > > > > > > > use that clone to iterate. There are issues reported > for > > >> > unsafe > > >> > > > > > > iterating > > >> > > > > > > > > of system properties in other projects as seen in [3]. > > >> > > > > > > > > > > >> > > > > > > > > Can we get this fixed in TomEE 1.7.3? What do I need > to > > do > > >> to > > >> > > get > > >> > > > > > this > > >> > > > > > > > in? > > >> > > > > > > > > > > >> > > > > > > > > Thanks. > > >> > > > > > > > > > > >> > > > > > > > > [1] > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > > https://github.com/apache/tomee/blob/tomee-1.7.x/container/openejb-loader/src/main/java/org/apache/openejb/loader/SystemInstance.java#L71 > > >> > > > > > > > > > > >> > > > > > > > > [2] > > >> > > > > > > > > org.apache.openejb.loader.LoaderRuntimeException: > Failed > > >> to > > >> > > > create > > >> > > > > > > > default > > >> > > > > > > > > instance of SystemInstance > > >> > > > > > > > > at > > >> > > > > > > > > > >> > > > > > > >> > > > org.apache.openejb.loader.SystemInstance.reset(SystemInstance.java:280) > > >> > > > > > > > > at > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > org.apache.openejb.loader.SystemInstance.<clinit>(SystemInstance.java:265) > > >> > > > > > > > > ... 9 more > > >> > > > > > > > > Caused by: java.util.ConcurrentModificationException > > >> > > > > > > > > at > > >> java.util.Hashtable$Enumerator.next(Hashtable.java:1167) > > >> > > > > > > > > at > > >> > > > > > > > > > >> > > > > > > >> > > > org.apache.openejb.loader.SystemInstance.<init>(SystemInstance.java:71) > > >> > > > > > > > > at > > >> > > > > > > > > > >> > > > > > > >> > > > org.apache.openejb.loader.SystemInstance.reset(SystemInstance.java:277) > > >> > > > > > > > > ... 10 more > > >> > > > > > > > > > > >> > > > > > > > > [3] > > >> > > > > > > > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=469706 > > >> > > > > > > > > https://liquibase.jira.com/browse/CORE-2104 > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > -- > > >> > > > > > > > Jonathan Gallimore > > >> > > > > > > > http://twitter.com/jongallimore > > >> > > > > > > > http://www.tomitribe.com > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > > >> > > > > >> > > -- > > >> > > ~~~*******'''''''''''''*******~~~ > > >> > > *Kasun Gajasinghe* > > >> > > Software Engineer; WSO2 Inc.; http://wso2.com, > > >> > > *linked-in: *http://lk.linkedin.com/in/gajasinghe > > >> > > *blog: **http://blog.kasunbg.org* <http://blog.kasunbg.org/> > > >> > > > > >> > > > > >> > > *twitter: **http://twitter.com/kasunbg* < > http://twitter.com/kasunbg > > > > > >> > > > > >> > > > >> > > >> > > >> > > >> -- > > >> ~~~*******'''''''''''''*******~~~ > > >> *Kasun Gajasinghe* > > >> Software Engineer; WSO2 Inc.; http://wso2.com, > > >> *linked-in: *http://lk.linkedin.com/in/gajasinghe > > >> *blog: **http://blog.kasunbg.org* <http://blog.kasunbg.org/> > > >> > > >> > > >> *twitter: **http://twitter.com/kasunbg* <http://twitter.com/kasunbg> > > >> > > > > > > > > > > > > -- > > > ~~~*******'''''''''''''*******~~~ > > > *Kasun Gajasinghe* > > > Software Engineer; WSO2 Inc.; http://wso2.com, > > > *linked-in: *http://lk.linkedin.com/in/gajasinghe > > > *blog: **http://blog.kasunbg.org* <http://blog.kasunbg.org/> > > > > > > > > > *twitter: **http://twitter.com/kasunbg* <http://twitter.com/kasunbg> > > > > > > > > > > > -- > > ~~~*******'''''''''''''*******~~~ > > *Kasun Gajasinghe* > > Software Engineer; WSO2 Inc.; http://wso2.com, > > *linked-in: *http://lk.linkedin.com/in/gajasinghe > > *blog: **http://blog.kasunbg.org* <http://blog.kasunbg.org/> > > > > > > *twitter: **http://twitter.com/kasunbg* <http://twitter.com/kasunbg> > > > > > > -- > Jonathan Gallimore > http://twitter.com/jongallimore > http://www.tomitribe.com > -- ~~~*******'''''''''''''*******~~~ *Kasun Gajasinghe* Software Engineer; WSO2 Inc.; http://wso2.com, *linked-in: *http://lk.linkedin.com/in/gajasinghe *blog: **http://blog.kasunbg.org* <http://blog.kasunbg.org/> *twitter: **http://twitter.com/kasunbg* <http://twitter.com/kasunbg>
