Scott Oaks wrote:
> If there are 1000s of clients accessing the application, does the list
> have all the client EMs? Or is there a different factory for each
> client? I'm a little concerned that cycling through the list could
> become expensive if there are that many simultaneous transactions -- and
> then the list becomes its own bottleneck.
>
The clients referring to the same Persistence Unit will share
EntityManagerFactory.
> I'm actually not sure about this entire thing: why can't the em.close()
> method remove itself from the list? Then the list wouldn't need to have
> WeakReferences at all, and when (if) the factory is closed, it can deal
> with the existing EMs.
>
Scott,
The EMF can be long living object. If an application is not well behaved
(that is it does not close ems) and we have a hard reference to it in
the list, then the ems would be around till the EMF is closed
Gordon,
As I mentioned in the previous mail, I am not sure why the list needs to
be maintained at all. The only consumer of the list is EMF#close() which
calls close() on ems that are still strongly referenced. Do you think we
would leak any resources if an emf is closed but corresponding ems are
not closed? Please note that with current approach, we are still not
calling close() for the ems that the app did not close and were no
longer strongly referenced.
Thanks,
Mitesh
> -Scott
>
> On Wed, 2006-08-30 at 14:28, Mitesh Meswani wrote:
>
>> Gordon Yorke wrote:
>>
>>> Hello Mitesh,
>>> This should be suffecient. We could move the code to createEntityManager instead of close(). That would ensure that the references are always cleaned up.
>>>
>>>
>> Good suggestion. It can help remove the synchronized blocks guarding
>> entityManagers. However, the list "entityManagers" would keep on growing
>> till a gc kicks in and we would be iterating the growing list every time
>> we create an entitymanager. With current approach for a well behaved
>> application, the list would be trimmed for each close. So, the choice is
>> between eliminating synchronized blocks or reducing iteration size.
>>
>> Scott,
>> Any suggestions on which approach might be better? Attaching the patch
>> again for your reference
>>
>> Thanks,
>> Mitesh
>>
>>
>>
>>> --Gordon
>>>
>>> -----Original Message-----
>>> From: Mitesh.Meswani_at_Sun.COM [mailto:Mitesh.Meswani_at_Sun.COM]On Behalf Of
>>> Mitesh Meswani
>>> Sent: Tuesday, August 29, 2006 8:58 PM
>>> To: Gordon Yorke; Tom Ware
>>> Cc: persistence_at_glassfish.dev.java.net
>>> Subject: Code review for issue 1003
>>>
>>>
>>> Hi Gordon,
>>>
>>> Please find attached a fix for issue 1003
>>> <https://glassfish.dev.java.net/issues/show_bug.cgi?id=1033>.
>>> Following is a summary:
>>> Changed the EntityManagerFactoryImpl.#entityManagers to be a linked list
>>> Added a new method emClosed() to EntityManagerFactoryImpl. This method
>>> is called from EntityManagerImpl.close(). The method iterates over the
>>> list and cleans up as required.
>>>
>>> Tests run:
>>> A local test exercising the code.
>>> Scott Oaks is running Specj with this code.
>>>
>>> This bug is one of the show stoppers for the UR1 release which has a HCF
>>> tomorrow (Wed). :(
>>>
>>> Please note that the fix has a limitation of not cleaning up the list in
>>> EntityManagerFactoryImpl if none of the ems of an application call
>>> close(). The implementation to solve it is a bit more involved. At this
>>> point I am wondering whether we need to maintain the list of
>>> entitymanagers at all in. I can implement the involved fix if
>>> maintaining the list is really required.
>>>
>>> Thanks,
>>> Mitesh
>>>
>>>
>>>
>>>