repo-sqale: fix of ExtItem/UriCache SQL connection leak

pspaude · Sep 14, 2021 · 6611883 · 6611883
1 parent efbab65
commit 6611883
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 33 deletions.
diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/ExtItemCache.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/ExtItemCache.java
@@ -33,6 +33,7 @@ public class ExtItemCache {
     private final Map<MExtItem.Key, MExtItem> keyToExtItem = new ConcurrentHashMap<>();
     private final Map<MExtItem.ItemNameKey, MExtItem> itemNameToExtItem = new ConcurrentHashMap<>();
 
+    // WARNING: Each .get() creates new connection, always use in try-with-resource block!
     private Supplier<JdbcSession> jdbcSessionSupplier;
 
     /**
@@ -105,14 +106,17 @@ private void updateMaps(MExtItem row) {
 
     private MExtItem retrieveFromDb(@NotNull MExtItem.Key key) {
         QExtItem ei = QExtItem.DEFAULT;
-        MExtItem row = jdbcSessionSupplier.get().newQuery()
-                .select(ei)
-                .from(ei)
-                .where(ei.itemName.eq(key.itemName))
-                .where(ei.valueType.eq(key.valueType))
-                .where(ei.holderType.eq(key.holderType))
-                .where(ei.cardinality.eq(key.cardinality))
-                .fetchOne();
+        MExtItem row;
+        try (JdbcSession jdbcSession = jdbcSessionSupplier.get().startReadOnlyTransaction()) {
+            row = jdbcSession.newQuery()
+                    .select(ei)
+                    .from(ei)
+                    .where(ei.itemName.eq(key.itemName))
+                    .where(ei.valueType.eq(key.valueType))
+                    .where(ei.holderType.eq(key.holderType))
+                    .where(ei.cardinality.eq(key.cardinality))
+                    .fetchOne();
+        }
         if (row != null) {
             updateMaps(row);
         }
@@ -129,14 +133,15 @@ private MExtItem retrieveFromDb(@NotNull MExtItem.Key key) {
             return extItem;
         }
 
-        extItem = jdbcSessionSupplier.get()
-                .newQuery()
-                .from(QExtItem.DEFAULT)
-                .select(QExtItem.DEFAULT)
-                .where(QExtItem.DEFAULT.itemName.eq(extItemKey.itemName)
-                        .and(QExtItem.DEFAULT.holderType.eq(extItemKey.holderType)))
-                // TODO let's consider fetchOne that throws if count > 1, right now we're not confident enough to do so.
-                .fetchFirst();
+        try (JdbcSession jdbcSession = jdbcSessionSupplier.get().startReadOnlyTransaction()) {
+            extItem = jdbcSession.newQuery()
+                    .from(QExtItem.DEFAULT)
+                    .select(QExtItem.DEFAULT)
+                    .where(QExtItem.DEFAULT.itemName.eq(extItemKey.itemName)
+                            .and(QExtItem.DEFAULT.holderType.eq(extItemKey.holderType)))
+                    // TODO let's consider fetchOne that throws if count > 1, right now we're not confident enough to do so.
+                    .fetchFirst();
+        }
 
         if (extItem != null) {
             updateMaps(extItem);
@@ -148,17 +153,18 @@ private MExtItem retrieveFromDb(@NotNull MExtItem.Key key) {
         if (jdbcSessionSupplier == null) {
             throw new IllegalStateException("Ext item cache was not initialized yet!");
         }
-        MExtItem extItem =  idToExtItem.get(id);
+        MExtItem extItem = idToExtItem.get(id);
         if (extItem != null) {
             return extItem;
         }
 
-        extItem = jdbcSessionSupplier.get()
-                .newQuery()
-                .from(QExtItem.DEFAULT)
-                .select(QExtItem.DEFAULT)
-                .where(QExtItem.DEFAULT.id.eq(id))
-                .fetchOne();
+        try (JdbcSession jdbcSession = jdbcSessionSupplier.get().startReadOnlyTransaction()) {
+            extItem = jdbcSession.newQuery()
+                    .from(QExtItem.DEFAULT)
+                    .select(QExtItem.DEFAULT)
+                    .where(QExtItem.DEFAULT.id.eq(id))
+                    .fetchOne();
+        }
 
         if (extItem != null) {
             updateMaps(extItem);

diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/UriCache.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/UriCache.java
@@ -67,6 +67,7 @@ public class UriCache {
     private final Map<Integer, String> idToUri = new ConcurrentHashMap<>();
     private final Map<String, Integer> uriToId = new ConcurrentHashMap<>();
 
+    // WARNING: Each .get() creates new connection, always use in try-with-resource block!
     private Supplier<JdbcSession> jdbcSessionSupplier;
 
     /**
@@ -224,11 +225,14 @@ private Integer retrieveId(String uri) {
     }
 
     private Integer retrieveIdFromDb(String uriString) {
-        MUri row = jdbcSessionSupplier.get().newQuery()
-            .select(QUri.DEFAULT)
-            .from(QUri.DEFAULT)
-            .where(QUri.DEFAULT.uri.eq(uriString))
-            .fetchOne();
+        MUri row;
+        try (JdbcSession jdbcSession = jdbcSessionSupplier.get().startReadOnlyTransaction()) {
+            row = jdbcSession.newQuery()
+                    .select(QUri.DEFAULT)
+                    .from(QUri.DEFAULT)
+                    .where(QUri.DEFAULT.uri.eq(uriString))
+                    .fetchOne();
+        }
         if (row == null) {
             return null;
         }
@@ -237,11 +241,14 @@ private Integer retrieveIdFromDb(String uriString) {
     }
 
     private String retrieveUriFromDb(Integer id) {
-        MUri row = jdbcSessionSupplier.get().newQuery()
-                .select(QUri.DEFAULT)
-                .from(QUri.DEFAULT)
-                .where(QUri.DEFAULT.id.eq(id))
-                .fetchOne();
+        MUri row;
+        try (JdbcSession jdbcSession = jdbcSessionSupplier.get().startReadOnlyTransaction()) {
+            row = jdbcSession.newQuery()
+                    .select(QUri.DEFAULT)
+                    .from(QUri.DEFAULT)
+                    .where(QUri.DEFAULT.id.eq(id))
+                    .fetchOne();
+        }
         if (row == null) {
             return null;
         }