# HG changeset patch
# User Andre Heinecke <andre.heinecke@intevation.de>
# Date 1424794574 -3600
# Node ID fe7e9da6312a80cad6f96262efe04fbcebc0a578
# Parent  ea17665c5aad714c16be14d7666de9763547af4f
(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

diff -r ea17665c5aad -r fe7e9da6312a artifacts/src/main/java/org/dive4elements/river/artifacts/context/SessionCallContextListener.java
--- 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();
     }
 }
diff -r ea17665c5aad -r fe7e9da6312a artifacts/src/main/java/org/dive4elements/river/artifacts/model/sq/MeasurementFactory.java
--- 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")
diff -r ea17665c5aad -r fe7e9da6312a artifacts/src/main/java/org/dive4elements/river/artifacts/services/BedKMChartService.java
--- 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();
         }
     }
diff -r ea17665c5aad -r fe7e9da6312a artifacts/src/main/java/org/dive4elements/river/artifacts/services/BedloadKMChartService.java
--- 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();
         }
     }
diff -r ea17665c5aad -r fe7e9da6312a artifacts/src/main/java/org/dive4elements/river/artifacts/services/D4EService.java
--- 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();
     }
 }
diff -r ea17665c5aad -r fe7e9da6312a artifacts/src/main/java/org/dive4elements/river/artifacts/services/DischargeTablesOverview.java
--- 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,
diff -r ea17665c5aad -r fe7e9da6312a artifacts/src/main/java/org/dive4elements/river/artifacts/services/FixingsKMChartService.java
--- 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();
         }
     }
diff -r ea17665c5aad -r fe7e9da6312a artifacts/src/main/java/org/dive4elements/river/artifacts/services/SQKMChartService.java
--- 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();
         }
     }
diff -r ea17665c5aad -r fe7e9da6312a backend/src/main/java/org/dive4elements/river/backend/SedDBSessionHolder.java
--- 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 :
diff -r ea17665c5aad -r fe7e9da6312a backend/src/main/java/org/dive4elements/river/backend/SessionHolder.java
--- 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();
     }
 }