Author: athimel Date: 2014-04-03 18:52:28 +0200 (Thu, 03 Apr 2014) New Revision: 3042 Url: http://forge.nuiton.org/projects/topia/repository/revisions/3042 Log: fixes #3140 Try to fix Hibernate connection not closed Modified: branches/topia-2.8.1.x/topia-persistence/src/main/java/org/nuiton/topia/framework/TopiaContextImpl.java branches/topia-2.8.1.x/topia-persistence/src/main/java/org/nuiton/topia/framework/TopiaUtil.java branches/topia-2.8.1.x/topia-persistence/src/test/java/org/nuiton/topia/framework/TopiaUtilTest.java Modified: branches/topia-2.8.1.x/topia-persistence/src/main/java/org/nuiton/topia/framework/TopiaContextImpl.java =================================================================== --- branches/topia-2.8.1.x/topia-persistence/src/main/java/org/nuiton/topia/framework/TopiaContextImpl.java 2014-04-03 15:59:40 UTC (rev 3041) +++ branches/topia-2.8.1.x/topia-persistence/src/main/java/org/nuiton/topia/framework/TopiaContextImpl.java 2014-04-03 16:52:28 UTC (rev 3042) @@ -64,15 +64,11 @@ import org.hibernate.boot.registry.StandardServiceRegistry; import org.hibernate.boot.registry.StandardServiceRegistryBuilder; import org.hibernate.cfg.Configuration; -import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider; import org.hibernate.event.service.spi.EventListenerRegistry; import org.hibernate.event.spi.EventType; -import org.hibernate.internal.SessionFactoryImpl; import org.hibernate.jdbc.Work; -import org.hibernate.service.Service; -import org.hibernate.service.ServiceRegistry; -import org.hibernate.service.ServiceRegistryBuilder; -import org.hibernate.service.spi.SessionFactoryServiceRegistry; +import org.hibernate.service.spi.Stoppable; import org.hibernate.tool.hbm2ddl.SchemaExport; import org.hibernate.tool.hbm2ddl.SchemaUpdate; import org.nuiton.topia.TopiaContext; @@ -504,69 +500,6 @@ return hibernate; } - /** - * Method to extract from the given Hibernate SessionFactory a working instance of StandardServiceRegistry - * <p/> - * IMPORTANT : As much as possible, prefer using the - * {@link #getSessionFactoryServiceRegistry(org.hibernate.SessionFactory)} mthod instead of the current one because - * the SessionFactoryServiceRegistry is a child of the StandardServiceRegistry - * <p/> - * NB: This method is static to make sure it does not depend on the current instance - * - * @param sessionFactory the Hibernate's SessionFactory instance - * @return the StandardServiceRegistry instance used by the given SessionFactory - */ - protected static StandardServiceRegistry getStandardServiceRegistry(SessionFactory sessionFactory) { - - // AThimel 03/04/14 The next two lines are the good way to get the StandardServiceRegistry in Hibernate 4.3 - SessionFactory.SessionFactoryOptions sessionFactoryOptions = sessionFactory.getSessionFactoryOptions(); - StandardServiceRegistry result = sessionFactoryOptions.getServiceRegistry(); - - return result; - } - - /** - * Method to extract from the given Hibernate SessionFactory a working instance of SessionFactoryServiceRegistry - * <p/> - * IMPORTANT : If possible, prefer using this method instead of - * {@link #getStandardServiceRegistry(org.hibernate.SessionFactory)} because the SessionFactoryServiceRegistry is a - * child of the StandardServiceRegistry - * <p/> - * NB: This method is static to make sure it does not depend on the current instance - * - * @param sessionFactory the Hibernate's SessionFactory instance - * @return the SessionFactoryServiceRegistry instance used by the given SessionFactory - */ - protected static SessionFactoryServiceRegistry getSessionFactoryServiceRegistry(SessionFactory sessionFactory) { - - // AThimel 03/04/14 The next two lines are the good way to get the SessionFactoryServiceRegistry in Hibernate 4.3 - SessionFactoryImplementor sessionFactoryImplementor = (SessionFactoryImplementor) sessionFactory; - SessionFactoryServiceRegistry result = (SessionFactoryServiceRegistry)sessionFactoryImplementor.getServiceRegistry(); - - return result; - } - - /** - * Method to get an Hibernate service instance from a given Hibernate SessionFactory - * <p/> - * NB: This method is static to make sure it does not depend on the current instance - * - * @param sessionFactory the Hibernate's SessionFactory instance - * @param serviceClass the expected service class - * @return the found service instance - * @throws org.hibernate.service.UnknownServiceException Indicates the service was not known. - * @see org.hibernate.service.ServiceRegistry#getService(Class) - */ - public static <S extends Service> S getHibernateService(SessionFactory sessionFactory, Class<S> serviceClass) { - - // Hibernate 4.3.x : prefer using the SessionFactoryServiceRegistry method instead of StandardServiceRegistry - // because SessionFactoryServiceRegistry is a child of the StandardServiceRegistry - ServiceRegistry serviceRegistry = getSessionFactoryServiceRegistry(sessionFactory); - - S result = serviceRegistry.getService(serviceClass); - return result; - } - @Override public SessionFactory getHibernateFactory() throws TopiaNotFoundException { if (hibernateFactory == null) { @@ -575,19 +508,12 @@ } else { // init service registry - // Hibernate 4.2 -// ServiceRegistry serviceRegistry = new ServiceRegistryBuilder().applySettings( -// properties).buildServiceRegistry(); -// -// hibernateFactory = getHibernateConfiguration().buildSessionFactory(serviceRegistry); - - // Hibernate 4.3 Properties properties = getHibernateConfiguration().getProperties(); StandardServiceRegistryBuilder builder = new StandardServiceRegistryBuilder(); StandardServiceRegistry standardServiceRegistry = builder.applySettings(properties).build(); hibernateFactory = getHibernateConfiguration().buildSessionFactory(standardServiceRegistry); - EventListenerRegistry eventListenerRegistry = getHibernateService(hibernateFactory, EventListenerRegistry.class); + EventListenerRegistry eventListenerRegistry = TopiaUtil.getHibernateService(hibernateFactory, EventListenerRegistry.class); TopiaFiresSupport.TopiaHibernateEvent listener = new TopiaFiresSupport.TopiaHibernateEvent(this); @@ -923,6 +849,15 @@ getParentContext().removeChildContext(this); } else { if (hibernateFactory != null) { + + // XXX AThimel 03/04/14 This is just a security, I'm not sure it is useful anymore + // close connection provider if possible (http://nuiton.org/issues/2757) + ConnectionProvider service = TopiaUtil.getHibernateService(hibernateFactory, ConnectionProvider.class); + if (service != null && service instanceof Stoppable) { + Stoppable stoppable = (Stoppable) service; + stoppable.stop(); + } + hibernateFactory.close(); closed = true; TopiaContextFactory.removeContext(this); @@ -942,10 +877,8 @@ // closed = true; // log.debug("TopiaContext finalized"); // } - if (!closed) { - - // means resource was not well closed, can't accept this! - throw new TopiaException("TopiaContext "+this+" was not closed!"); + if (!closed && log.isErrorEnabled()) { + log.error("TopiaContext "+this+" was not closed!"); } super.finalize(); } Modified: branches/topia-2.8.1.x/topia-persistence/src/main/java/org/nuiton/topia/framework/TopiaUtil.java =================================================================== --- branches/topia-2.8.1.x/topia-persistence/src/main/java/org/nuiton/topia/framework/TopiaUtil.java 2014-04-03 15:59:40 UTC (rev 3041) +++ branches/topia-2.8.1.x/topia-persistence/src/main/java/org/nuiton/topia/framework/TopiaUtil.java 2014-04-03 16:52:28 UTC (rev 3042) @@ -32,10 +32,12 @@ import org.hibernate.cfg.Configuration; import org.hibernate.dialect.Dialect; import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider; +import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.mapping.PersistentClass; import org.hibernate.mapping.Table; +import org.hibernate.service.Service; import org.hibernate.service.ServiceRegistry; -import org.hibernate.service.ServiceRegistryBuilder; +import org.hibernate.service.spi.SessionFactoryServiceRegistry; import org.hibernate.tool.hbm2ddl.DatabaseMetadata; import org.hibernate.tool.hbm2ddl.TableMetadata; import org.nuiton.topia.TopiaContext; @@ -55,7 +57,6 @@ import java.util.StringTokenizer; import java.util.regex.Pattern; -import com.google.common.base.Preconditions; import com.google.common.base.Supplier; /** @@ -181,10 +182,14 @@ public static boolean isSchemaExist(TopiaContext tx, String entityName) { + TopiaContextImplementor txi = (TopiaContextImplementor) tx; + + ConnectionProviderSupplier connectionProviderSupplier = null; + boolean exist = false; try { - TopiaContextImplementor txi = (TopiaContextImplementor) tx; + connectionProviderSupplier = new ConnectionProviderSupplier(txi); Configuration configuration = txi.getHibernateConfiguration(); PersistentClass classMapping = @@ -206,8 +211,7 @@ "could not find entity with name " + entityName); } - ConnectionProvider connectionProvider = - getConnectionProvider(configuration); + ConnectionProvider connectionProvider = connectionProviderSupplier.get(); Dialect dialect = Dialect.getDialect(configuration.getProperties()); @@ -235,6 +239,14 @@ log.error("Cant connect to database", e); } catch (TopiaNotFoundException e) { log.error("Cant connect to database", e); + } finally { + if (connectionProviderSupplier != null) { + try { + connectionProviderSupplier.close(); + } catch (IOException e) { + log.error("Cant close connection provider", e); + } + } } return exist; @@ -251,6 +263,9 @@ public static boolean isSchemaExist(Configuration configuration, String entityName) { + ConnectionProviderSupplier connectionProviderSupplier = + new ConnectionProviderSupplier(configuration); + boolean exist = false; try { @@ -273,8 +288,7 @@ "could not find entity with name " + entityName); } - ConnectionProvider connectionProvider = - getConnectionProvider(configuration); + ConnectionProvider connectionProvider = connectionProviderSupplier.get(); Dialect dialect = Dialect.getDialect(configuration.getProperties()); @@ -300,6 +314,12 @@ } catch (SQLException e) { log.error("Cant connect to database", e); + } finally { + try { + connectionProviderSupplier.close(); + } catch (IOException e) { + log.error("Cant close connection provider", e); + } } return exist; @@ -316,10 +336,12 @@ */ public static boolean isSchemaEmpty(Configuration configuration) { + ConnectionProviderSupplier connectionProviderSupplier = + new ConnectionProviderSupplier(configuration); + try { - ConnectionProvider connectionProvider = - getConnectionProvider(configuration); + ConnectionProvider connectionProvider = connectionProviderSupplier.get(); Dialect dialect = Dialect.getDialect(configuration.getProperties()); @@ -368,20 +390,129 @@ } catch (SQLException e) { log.error("Cant connect to database", e); + } finally { + try { + connectionProviderSupplier.close(); + } catch (IOException e) { + log.error("Cant close connection provider", e); + } } return true; } - protected static ConnectionProvider getConnectionProvider(Configuration configuration) { - Properties properties = configuration.getProperties(); - StandardServiceRegistryBuilder builder = new StandardServiceRegistryBuilder(); - ServiceRegistry serviceRegistry = builder.applySettings(properties).build(); - // FIXME AThimel 03/04/14 serviceRegistry must be closed - return serviceRegistry.getService(ConnectionProvider.class); + /** + * Method to extract from the given Hibernate SessionFactory a working instance of StandardServiceRegistry + * <p/> + * IMPORTANT : As much as possible, prefer using the + * {@link #getSessionFactoryServiceRegistry(org.hibernate.SessionFactory)} mthod instead of the current one because + * the SessionFactoryServiceRegistry is a child of the StandardServiceRegistry + * <p/> + * NB: This method is static to make sure it does not depend on the current instance + * + * @param sessionFactory the Hibernate's SessionFactory instance + * @return the StandardServiceRegistry instance used by the given SessionFactory + */ + protected static StandardServiceRegistry getStandardServiceRegistry(SessionFactory sessionFactory) { + + // AThimel 03/04/14 The next two lines are the good way to get the StandardServiceRegistry in Hibernate 4.3 + SessionFactory.SessionFactoryOptions sessionFactoryOptions = sessionFactory.getSessionFactoryOptions(); + StandardServiceRegistry result = sessionFactoryOptions.getServiceRegistry(); + + return result; } /** + * Method to extract from the given Hibernate SessionFactory a working instance of SessionFactoryServiceRegistry + * <p/> + * IMPORTANT : If possible, prefer using this method instead of + * {@link #getStandardServiceRegistry(org.hibernate.SessionFactory)} because the SessionFactoryServiceRegistry is a + * child of the StandardServiceRegistry + * <p/> + * NB: This method is static to make sure it does not depend on the current instance + * + * @param sessionFactory the Hibernate's SessionFactory instance + * @return the SessionFactoryServiceRegistry instance used by the given SessionFactory + */ + protected static SessionFactoryServiceRegistry getSessionFactoryServiceRegistry(SessionFactory sessionFactory) { + + // AThimel 03/04/14 The next two lines are the good way to get the SessionFactoryServiceRegistry in Hibernate 4.3 + SessionFactoryImplementor sessionFactoryImplementor = (SessionFactoryImplementor) sessionFactory; + SessionFactoryServiceRegistry result = (SessionFactoryServiceRegistry)sessionFactoryImplementor.getServiceRegistry(); + + return result; + } + + /** + * Method to get an Hibernate service instance from a given Hibernate SessionFactory + * <p/> + * NB: This method is static to make sure it does not depend on the current instance + * + * @param sessionFactory the Hibernate's SessionFactory instance + * @param serviceClass the expected service class + * @return the found service instance + * @throws org.hibernate.service.UnknownServiceException Indicates the service was not known. + * @see org.hibernate.service.ServiceRegistry#getService(Class) + */ + public static <S extends Service> S getHibernateService(SessionFactory sessionFactory, Class<S> serviceClass) { + + // Hibernate 4.3.x : prefer using the SessionFactoryServiceRegistry method instead of StandardServiceRegistry + // because SessionFactoryServiceRegistry is a child of the StandardServiceRegistry + ServiceRegistry serviceRegistry = getSessionFactoryServiceRegistry(sessionFactory); + + S result = serviceRegistry.getService(serviceClass); + return result; + } + + /** + * Hibernate 4.3.x compatible Supplier<ConnectionProvider>. The provider will choose the best way to find the + * ConnectionProvider depending on the way is has been created. + */ + public static class ConnectionProviderSupplier implements Supplier<ConnectionProvider>, Closeable { + + /** + * StandardServiceRegistry will be used if no SessionFactory is provided + */ + protected StandardServiceRegistry standardServiceRegistry; + + protected ConnectionProvider connectionProvider; + + protected boolean embeddedRegistry; + + public ConnectionProviderSupplier(TopiaContextImplementor topiaContextImplementor) throws TopiaNotFoundException { + this(topiaContextImplementor.getHibernateConfiguration()); + embeddedRegistry = false; + } + + public ConnectionProviderSupplier(Configuration configuration) { + Properties properties = configuration.getProperties(); + StandardServiceRegistryBuilder builder = new StandardServiceRegistryBuilder(); + this.standardServiceRegistry = builder.applySettings(properties).build(); + embeddedRegistry = true; + } + + @Override + public ConnectionProvider get() { + if (connectionProvider == null) { + // otherwise use the StandardServiceRegistry + connectionProvider = standardServiceRegistry.getService(ConnectionProvider.class); + } + return connectionProvider; + } + + @Override + public void close() throws IOException { + // Do not close the SessionFactory, it is probably used somewhere else + + // On the over hand, if standardServiceRegistry is provided, that means the its has been created explicitly + // for the current instance, close it + if (standardServiceRegistry != null && embeddedRegistry) { + StandardServiceRegistryBuilder.destroy(standardServiceRegistry); + } + } + } + + /** * Return hibernate schema name * * @param config of hibernate Modified: branches/topia-2.8.1.x/topia-persistence/src/test/java/org/nuiton/topia/framework/TopiaUtilTest.java =================================================================== --- branches/topia-2.8.1.x/topia-persistence/src/test/java/org/nuiton/topia/framework/TopiaUtilTest.java 2014-04-03 15:59:40 UTC (rev 3041) +++ branches/topia-2.8.1.x/topia-persistence/src/test/java/org/nuiton/topia/framework/TopiaUtilTest.java 2014-04-03 16:52:28 UTC (rev 3042) @@ -103,7 +103,8 @@ // FIXME echatellier 20130315 ce test fail depuis probablement a cause // de hibernate.hbm2ddl.auto=update, à confirmer... - assertFalse(actual); + + assertFalse("Schema is not supposed to exist, but isSchemaExist=" + actual, actual); TopiaContext tx = rootContext.beginTransaction(); tx.createSchema();