> In my case it wasn't something I was aware of -- the application (from a > vendor) implements Spring 2.0's AbstractEntityManagerFactoryBean such that > it calls PersistenceProviderImpl.createEntityManagerFactory(String name, > String resource, Map m), which seems to use the finalizing broker by > default.
Bummer. There is a Spring bean that uses the createContainerEntityManagerFactory() signature; too bad that the application doesn't use it. > I assume that since I ended up with a memory leak, Spring 2.0's cleanup > practices are dubious, and the finalizing broker is necessary. FWIW, I don't know enough about the semantics of AbstractEntityManagerFactoryBean to know whether Spring should be expected to clean up EMFs obtained from it. > So am I correct in guessing that the non-finalizing broker should be the one > in the ConcurrentHashMap (with hard references), and the finalizing broker > should be in the weak reference set? I haven't read through the older parts of the thread enough to be sure, and don't have the source code handy right now, so I don't have a good answer for you right now. -Patrick On Wed, Jul 22, 2009 at 5:55 PM, David Minor<[email protected]> wrote: > Patrick, many thanks for replying. > > In my case it wasn't something I was aware of -- the application (from a > vendor) implements Spring 2.0's AbstractEntityManagerFactoryBean such that > it calls PersistenceProviderImpl.createEntityManagerFactory(String name, > String resource, Map m), which seems to use the finalizing broker by > default. > > I assume that since I ended up with a memory leak, Spring 2.0's cleanup > practices are dubious, and the finalizing broker is necessary. > > So am I correct in guessing that the non-finalizing broker should be the one > in the ConcurrentHashMap (with hard references), and the finalizing broker > should be in the weak reference set? > > >> -----Original Message----- >> From: Patrick Linskey [mailto:[email protected]] >> Sent: Wednesday, July 22, 2009 4:58 PM >> To: [email protected] >> Subject: Re: FW: Memory leak >> >> Hi, >> >> Is there any particular reason why you need the finalizing broker? One >> (vastly preferable) option would be to use the non-finalizing broker, which, >> of course, means that you're promising to close your brokers at the >> appropriate lifecycle moments. >> >> Certainly, the bug deserves to be addressed, as the whole point of the >> finalizing mode is to be robust in an environment with dubious cleanup >> practices. >> >> -Patrick >> >> On Wed, Jul 22, 2009 at 4:15 PM, Russell Collins< >> [email protected]> wrote: >> > Is there a configuration setting or work around for this in the meantime? >> > >> > >> > Russell Collins >> > Sr. Software Engineer >> > McLane Advanced Technology >> > >> > "Do or do not, there is no try." - Yoda >> > >> > >> > -----Original Message----- >> > From: David Minor [mailto:[email protected]] >> > Sent: Wednesday, July 22, 2009 11:12 AM >> > To: [email protected]; [email protected] >> > Subject: Re: FW: Memory leak >> > >> > I've created http://issues.apache.org/jira/browse/OPENJPA-1193 >> > >> > One thought I had was that the if statement might simply be missing a >> > ! -- since this is a concurrency improvement for non-finalizing >> > brokers, perhaps the non-finalizing brokers were supposed to be >> > switched to the ConcurrentHashMap, rather than the finalizing brokers. >> > >> > Is Patrick Linskey still involved in openjpa? I haven't seen his name >> > for awhile. >> > >> > >> >> -----Original Message----- >> >> From: Michael Dick [mailto:[email protected]] >> >> Sent: Wednesday, July 22, 2009 6:47 AM >> >> To: [email protected]; [email protected] >> >> Subject: Re: FW: Memory leak >> >> >> >> Hi David, >> >> >> >> At the moment I don't have a ton of free time to dive into the change >> >> or the peformance bottleneck that it resolved. In the interest of >> >> providing some relief (trunk & 1.3.x) I'd be happy to make this >> >> change configurable (maybe something on openjpa.conf.Compatibility) >> >> so you can get the old behavior. >> >> >> >> I've cross posted to d...@openjpa to see if there are any objections. >> >> >> >> In any event would you mind opening a JIRA issue (I'd do it but then >> >> you have to subscribe to get notifications etc.). >> >> >> >> On Tue, Jul 21, 2009 at 6:37 PM, David Minor <[email protected]> >> >> wrote: >> >> >> >> > Looks like this code was checked in a year ago by Patrick Linskey >> >> > with >> >> >> >> > the comment "Improve concurrency by actively managing >> >> > AbstractBrokerFactory's broker set when using non-finalizing brokers. >> >> > Credit goes to Arunabh Hazarika for identifying the bottleneck and >> >> prototyping this solution." >> >> > >> >> > I'm not sure why _brokers was changed with regards to >> >> > FinalizingBrokerImpl though. BrokerImpl's free() method was >> >> > modified to call the factory to remove the it from _brokers. >> >> > FinalizingBrokerImpl calls free() from its finalizer, but the >> >> > finalizer will never be called if there is a reference in _brokers. >> >> > >> >> > Anyone have any ideas? >> >> > >> >> > On Tue, Jul 21, 2009 at 11:24 AM, David Minor <[email protected]> >> >> wrote: >> >> > >> >> > > Hi Mike, >> >> > > >> >> > > I've tracked the problem down to >> >> > > org.apache.openjpa.kernel.AbstractBrokerFactory. The heap dump >> >> > > lists >> >> >> >> > > the _brokers Set as containing all the references to >> >> > > FinalizingBrokerImpl, >> >> > and >> >> > > it appears the assignment of this set was changed to this: >> >> > > >> >> > > if (FinalizingBrokerImpl.class.isAssignableFrom( >> >> > > bv.getTemplateBrokerType(_conf))) { >> >> > > return MapBackedSet.decorate(new ConcurrentHashMap(), >> >> > > new Object() { }); >> >> > > } else { >> >> > > return new ConcurrentReferenceHashSet( >> >> > > ConcurrentReferenceHashSet.WEAK); >> >> > > } >> >> > > >> >> > > It used to be assigned to the weak reference hash set as in the >> >> > > else >> >> >> >> > > statement. Forcing the second assignment fixes the problem. >> >> > > >> >> > > >> >> > > > >> >> > > > -----Original Message----- >> >> > > > From: Michael Dick [mailto:[email protected]] >> >> > > > Sent: Tuesday, July 14, 2009 7:29 AM >> >> > > > To: [email protected] >> >> > > > Subject: Re: FW: Memory leak >> >> > > > >> >> > > > Hi David, >> >> > > > >> >> > > > There have been a few changes in PersistenceProviderImpl. One >> >> > > > was to make the non-finalizing BrokerImpl the default (must be >> >> > > > overridden in your >> >> > > > config) another that might be interesting was adding a pool of >> >> > > > EntityManagerFactories. >> >> > > > >> >> > > > From what I've seen the EMF pool is not used by default, but >> >> > > > it's possible that the AbstractEntityManagerFactoryBean is >> >> > > > setting it (the property is named EntityManagerFactoryPool in >> >> > > > case that helps). I took a quick pass at setting the pooling >> >> > > > property and the only way I saw it take effect was to pass it >> >> > > > in as a JVM arg (might be something in my eclipse env though - >> >> > > > and I'm on >> >> 2.0.0-SNAPSHOT ATM). >> >> > > > >> >> > > > Hope this gives you a starting point, if not keep replying and >> >> > > > we'll >> >> > try >> >> > > > to help >> >> > > > >> >> > > > -mike >> >> > > > >> >> > > > On Mon, Jul 13, 2009 at 8:00 PM, David Minor >> >> > > > <[email protected]> >> >> > > > wrote: >> >> > > > >> >> > > >> Hi Mike, >> >> > > >> >> >> > > >> Nothing else has changed. The application extends spring 2.0's >> >> > > >> AbstractEntityManagerFactoryBean class (apparently so that the >> >> > > >> persistence.xml file can be named something different). >> >> > > >> >> >> > > >> I notice it is checking the return type of >> >> > > >> AbstractEntityManagerFactoryBean's getPersistenceProvider() >> >> > > >> for an instance of openjpa's PersistenceProviderImpl, and >> >> > > >> doing something different depending on whether it finds it or >> >> > > >> not. Has anything changed with regards to this class? >> >> > > >> >> >> > > >> > -----Original Message----- >> >> > > >> > From: Michael Dick [mailto:[email protected]] >> >> > > >> > Sent: Monday, July 13, 2009 12:49 PM >> >> > > >> > To: [email protected] >> >> > > >> > Subject: Re: Memory leak >> >> > > >> > >> >> > > >> > Hi David, >> >> > > >> > >> >> > > >> > FinalizingBrokerImpl will close itself and free resources >> >> > > >> > when it's GC'ed. >> >> > > >> > It sounds like something else is holding on to a lot of >> >> > > >> > references to FBImpl (I'd guess something changed "upstream"). >> >> > > >> > >> >> > > >> > One cause is if the application creates a large number of >> >> > > >> > EntityManagers and doesn't close them (or creates a large >> >> > > >> > number of EMFactories which don't get closed since closing >> >> > > >> > an EMF will close >> >> > > > its EMs). >> >> > > >> > >> >> > > >> > Did anything else change or did you just upgrade OpenJPA >> >> versions? >> >> > > >> > >> >> > > >> > -mike >> >> > > >> > >> >> > > >> > On Mon, Jul 13, 2009 at 11:34 AM, David Minor >> >> > > >> > <[email protected]> >> >> > > >> > wrote: >> >> > > >> > >> >> > > >> >> Upgrading openjpa from 1.0.1 to 1.2.1 seems to introduce a >> >> > > >> >> memory leak >> >> > > >> > >> >> > > >> >> in our application -- leaving the server running for a few >> >> > > >> >> days results in OOM errors (there are quartz tasks making >> >> > > >> >> simple openjpa >> >> > > > >> >> > > >> >> selects during this time). A heap dump reveals >> >> > > >> >> org.apache.openjpa.kernel.FinalizingBrokerImpl as the >> >> > > >> >> dominant >> >> >> >> > > >> >> object, >> >> > > >> > >> >> > > >> >> according to Eclipse's memory analysis plugin. >> >> > > >> >> >> >> > > >> >> Does anyone have an idea of what might be causing this? >> >> > > >> >> >> >> > > >> >> -- >> >> > > >> >> _____________ >> >> > > >> >> David Minor >> >> > > >> >> >> >> > > >> > >> >> > > >> > >> >> > > >> > >> >> > > >> > >> >> > > >> >> >> > > >> >> >> > > >> >> >> > > >> -- >> >> > > >> _____________ >> >> > > >> David Minor >> >> > > >> >> >> > > > >> >> > > > >> >> > > > >> >> > > > >> >> > > >> >> > > >> >> > > >> >> > > -- >> >> > > _____________ >> >> > > David Minor >> >> > > >> >> > > >> >> > >> >> > >> >> > -- >> >> > _____________ >> >> > David Minor >> >> > >> >> >> >> >> >> >> >> >> > >> > >> > -- >> > _____________ >> > David Minor >> > >> >> >> >> -- >> Patrick Linskey >> 202 669 5907 >> >> >> >> > > > -- > _____________ > David Minor > -- Patrick Linskey 202 669 5907
