Mercurial > dive4elements > river

changeset 8566:fe7e9da6312a

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
(issue1797) Move close into session release and guard it. To avoid leaking open and Held sessions we centrally close the session in the SessionHolder on release. This also removes some duplicated code in SedDBSessionHolder
author Andre Heinecke <andre.heinecke@intevation.de>
date Tue, 24 Feb 2015 17:16:14 +0100
parents ea17665c5aad
children 9bbce0edd36b
files artifacts/src/main/java/org/dive4elements/river/artifacts/context/SessionCallContextListener.java artifacts/src/main/java/org/dive4elements/river/artifacts/model/sq/MeasurementFactory.java artifacts/src/main/java/org/dive4elements/river/artifacts/services/BedKMChartService.java artifacts/src/main/java/org/dive4elements/river/artifacts/services/BedloadKMChartService.java artifacts/src/main/java/org/dive4elements/river/artifacts/services/D4EService.java artifacts/src/main/java/org/dive4elements/river/artifacts/services/DischargeTablesOverview.java artifacts/src/main/java/org/dive4elements/river/artifacts/services/FixingsKMChartService.java artifacts/src/main/java/org/dive4elements/river/artifacts/services/SQKMChartService.java backend/src/main/java/org/dive4elements/river/backend/SedDBSessionHolder.java backend/src/main/java/org/dive4elements/river/backend/SessionHolder.java
diffstat 10 files changed, 26 insertions(+), 49 deletions(-) [+]
line wrap: on
line diff
--- a/artifacts/src/main/java/org/dive4elements/river/artifacts/context/SessionCallContextListener.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/artifacts/src/main/java/org/dive4elements/river/artifacts/context/SessionCallContextListener.java	Tue Feb 24 17:16:14 2015 +0100
@@ -67,9 +67,6 @@
     public void close(CallContext context) {
         log.debug("SessionCallContextListener.close");
 
-        Session session = (Session)context.getContextValue(SESSION_KEY);
-        session.close();
-
         SessionHolder.release();
     }
 }
--- a/artifacts/src/main/java/org/dive4elements/river/artifacts/model/sq/MeasurementFactory.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/artifacts/src/main/java/org/dive4elements/river/artifacts/model/sq/MeasurementFactory.java	Tue Feb 24 17:16:14 2015 +0100
@@ -258,18 +258,13 @@
         SQ.Factory sqFactory
     ) {
         Session session = SedDBSessionHolder.HOLDER.get();
-        try {
-            List<Measurement> totals = loadTotals(
-                session, river, location, dateRange);
+        List<Measurement> totals = loadTotals(
+            session, river, location, dateRange);
 
-            List<Measurement> accumulated = loadFractions(
-                session, river, location, dateRange);
+        List<Measurement> accumulated = loadFractions(
+            session, river, location, dateRange);
 
-            return new Measurements(totals, accumulated, sqFactory);
-        }
-        finally {
-            session.close();
-        }
+        return new Measurements(totals, accumulated, sqFactory);
     }
 
     @SuppressWarnings("unchecked")
--- a/artifacts/src/main/java/org/dive4elements/river/artifacts/services/BedKMChartService.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/artifacts/src/main/java/org/dive4elements/river/artifacts/services/BedKMChartService.java	Tue Feb 24 17:16:14 2015 +0100
@@ -134,7 +134,6 @@
             return doProcess(data, globalContext, callMeta);
         }
         finally {
-            SedDBSessionHolder.HOLDER.get().close();
             SedDBSessionHolder.release();
         }
     }
--- a/artifacts/src/main/java/org/dive4elements/river/artifacts/services/BedloadKMChartService.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/artifacts/src/main/java/org/dive4elements/river/artifacts/services/BedloadKMChartService.java	Tue Feb 24 17:16:14 2015 +0100
@@ -134,7 +134,6 @@
             return doProcess(data, globalContext, callMeta);
         }
         finally {
-            SedDBSessionHolder.HOLDER.get().close();
             SedDBSessionHolder.release();
         }
     }
--- a/artifacts/src/main/java/org/dive4elements/river/artifacts/services/D4EService.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/artifacts/src/main/java/org/dive4elements/river/artifacts/services/D4EService.java	Tue Feb 24 17:16:14 2015 +0100
@@ -60,9 +60,6 @@
     /** Called when processing done, close session. */
     protected void shutdown() {
         log.debug("shutdown");
-        Session session = SessionHolder.HOLDER.get();
-        session.close();
-
         SessionHolder.release();
     }
 }
--- a/artifacts/src/main/java/org/dive4elements/river/artifacts/services/DischargeTablesOverview.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/artifacts/src/main/java/org/dive4elements/river/artifacts/services/DischargeTablesOverview.java	Tue Feb 24 17:16:14 2015 +0100
@@ -77,10 +77,7 @@
 
     @Override
     protected void finish() {
-        if (session != null) {
-            session.close();
-            SessionHolder.release();
-        }
+        SessionHolder.release();
     }
 
     protected JFreeChart createChart(Document data,
--- a/artifacts/src/main/java/org/dive4elements/river/artifacts/services/FixingsKMChartService.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/artifacts/src/main/java/org/dive4elements/river/artifacts/services/FixingsKMChartService.java	Tue Feb 24 17:16:14 2015 +0100
@@ -193,7 +193,6 @@
             return doProcess(data, globalContext, callMeta);
         }
         finally {
-            SessionHolder.HOLDER.get().close();
             SessionHolder.release();
         }
     }
--- a/artifacts/src/main/java/org/dive4elements/river/artifacts/services/SQKMChartService.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/artifacts/src/main/java/org/dive4elements/river/artifacts/services/SQKMChartService.java	Tue Feb 24 17:16:14 2015 +0100
@@ -147,7 +147,6 @@
             return doProcess(data, globalContext, callMeta);
         }
         finally {
-            SedDBSessionHolder.HOLDER.get().close();
             SedDBSessionHolder.release();
         }
     }
--- a/backend/src/main/java/org/dive4elements/river/backend/SedDBSessionHolder.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/backend/src/main/java/org/dive4elements/river/backend/SedDBSessionHolder.java	Tue Feb 24 17:16:14 2015 +0100
@@ -13,39 +13,16 @@
 import org.hibernate.Session;
 import org.hibernate.SessionFactory;
 
-public class SedDBSessionHolder
+public class SedDBSessionHolder extends SessionHolder
 {
     private static Logger log =
         Logger.getLogger(SedDBSessionHolder.class);
 
-    public static final ThreadLocal<Session> HOLDER =
-        new ThreadLocal<Session>() {
-            @Override
-            protected Session initialValue() {
-                return create();
-            }
-        };
-
-    private SedDBSessionHolder() {
-    }
-
     public synchronized static Session create() {
         log.debug("create");
         SessionFactory sessionFactory =
             SessionFactoryProvider.getSedDBSessionFactory();
         return sessionFactory.openSession();
     }
-
-    public static Session acquire() {
-        log.debug("acquire");
-        Session session = create();
-        HOLDER.set(session);
-        return session;
-    }
-
-    public static void release() {
-        log.debug("release");
-        HOLDER.remove();
-    }
 }
 // vim:set ts=4 sw=4 si et sta sts=4 fenc=utf8 :
--- a/backend/src/main/java/org/dive4elements/river/backend/SessionHolder.java	Tue Feb 17 13:58:23 2015 +0100
+++ b/backend/src/main/java/org/dive4elements/river/backend/SessionHolder.java	Tue Feb 24 17:16:14 2015 +0100
@@ -27,7 +27,7 @@
             }
         };
 
-    private SessionHolder() {
+    protected SessionHolder() {
     }
 
     public synchronized static Session create() {
@@ -46,6 +46,24 @@
 
     public static void release() {
         log.debug("release");
+        Session session = HOLDER.get();
+        if (session != null) {
+            try {
+                if (!session.isOpen()) {
+                    /* If this check really works in all cases why does hibernate
+                     * does not use it when you call close anyway. */
+                    log.debug("Session is not open. Calling close anyway.");
+                }
+                session.close();
+            } catch (Exception e) {
+                /* Do not try this at home. But hibernate may fail with an NPE
+                 * or any other exception depending on the state of the session.
+                 * According to doc this should only throw a HibernateException but
+                 * we definetely get NPE's here.
+                 */
+                log.error("Exception caught on session close! Session already closed?" + e.getMessage());
+            }
+        }
         HOLDER.remove();
     }
 }
http://dive4elements.wald.intevation.org