persistence@glassfish.java.net

Re: Code review for: [Issue 1107] Logger defined by a property doesn't work]

From: Marina Vatkina <Marina.Vatkina_at_Sun.COM>
Date: Thu, 12 Oct 2006 11:49:00 -0700

Hi Wonseok,

Yes, I was also thinking about that, but clone() is a shallow copy, and
the 2 loggers will share all the maps defined in JavaLog between them,
which can cause a memory leak as when the PU is unloaded, it's loggers
will still be registered in the maps.

What do you think?

thanks,
-marina

Wonseok Kim wrote:
> Hi Marina,
>
> This is another issue but could you modify updateLoggers() also?
> I found that we don't need to instantiate two SessionLog instances
> because Session.setSessionLog() does clone its parameter always.
>
> The following is modified code:
>
> protected void updateLoggers(AbstractSession session, Map m,
> ServerPlatform serverPlatform){
> // Logger(SessionLog type) can be specified by the logger
> property or ServerPlatform.getServerLog().
> // The logger property has a higher priority to
> ServerPlatform.getServerLog().
>
> String loggerClassName =
> PropertiesHandler.getPropertyValueLogDebug(TopLinkProperties.LOGGING_LOGGER,
> m, session);
> Class sessionLogClass = null;
> if (loggerClassName != null) {
> sessionLogClass = findClassForProperty(loggerClassName,
> TopLinkProperties.LOGGING_LOGGER);
> }
>
> SessionLog sessionLog = null;
> if(sessionLogClass != null){
> try {
> sessionLog = (SessionLog)sessionLogClass.newInstance();
> } catch (Exception ex) {
> throw
> EntityManagerSetupException.failedToInstantiateLogger (loggerClassName,
> TopLinkProperties.LOGGING_LOGGER, ex);
> }
> } else if(serverPlatform != null){
> // if ServerPlatform.getServerLog () returns null, we won't
> set loggers
> sessionLog = serverPlatform.getServerLog();
> }
>
> // Set loggers for the singleton logger and the session logger.
> // Don't change default loggers if null
> if (sessionLog != null){
> AbstractSessionLog.setLog(sessionLog);
> session.setSessionLog (sessionLog); // sessionLog instance
> won't be shared because it's cloned internally.
> }
>
> //Bug5389828. Update the logging settings for the singleton
> logger.
> initOrUpdateLogging(true, m, AbstractSessionLog.getLog());
> initOrUpdateLogging(true, m, session.getSessionLog());
> }
>
> Thanks
> - Wonseok
>