tomee-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rmannibu...@apache.org
Subject tomee git commit: TOMEE-1857 avoid Logger.getInstance to leak
Date Mon, 04 Jul 2016 06:42:48 GMT
Repository: tomee
Updated Branches:
  refs/heads/master fc14d71df -> c38893cb9


TOMEE-1857 avoid Logger.getInstance to leak


Project: http://git-wip-us.apache.org/repos/asf/tomee/repo
Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/c38893cb
Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/c38893cb
Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/c38893cb

Branch: refs/heads/master
Commit: c38893cb954b9c5ad7af030025d7951505f759a7
Parents: fc14d71
Author: Romain manni-Bucau <rmannibucau@gmail.com>
Authored: Mon Jul 4 08:42:29 2016 +0200
Committer: Romain manni-Bucau <rmannibucau@gmail.com>
Committed: Mon Jul 4 08:42:29 2016 +0200

----------------------------------------------------------------------
 .../org/apache/openejb/util/LogCategory.java    | 11 +++-
 .../java/org/apache/openejb/util/Logger.java    | 52 ++++++++++++++----
 .../java/org/apache/openejb/util/Memoizer.java  |  4 ++
 .../org/apache/openejb/util/MemoizerTest.java   | 55 ++++++++++++++++++++
 4 files changed, 110 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tomee/blob/c38893cb/container/openejb-core/src/main/java/org/apache/openejb/util/LogCategory.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/util/LogCategory.java
b/container/openejb-core/src/main/java/org/apache/openejb/util/LogCategory.java
index 49b3ffd..e1491cd 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/util/LogCategory.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/util/LogCategory.java
@@ -61,7 +61,7 @@ public final class LogCategory {
     public static final LogCategory OPENEJB_SQL = OPENEJB.createChild("sql");
 
     private LogCategory(final String name) {
-        this.name = name;
+        this.name = name == null ? "" : name;
     }
 
     public String getName() {
@@ -80,4 +80,13 @@ public final class LogCategory {
         return new LogCategory(this.name + "." + child);
     }
 
+    @Override
+    public boolean equals(final Object o) {
+        return this == o || !(o == null || getClass() != o.getClass()) && name.equals(LogCategory.class.cast(o).name);
+    }
+
+    @Override
+    public int hashCode() {
+        return name.hashCode();
+    }
 }

http://git-wip-us.apache.org/repos/asf/tomee/blob/c38893cb/container/openejb-core/src/main/java/org/apache/openejb/util/Logger.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/util/Logger.java b/container/openejb-core/src/main/java/org/apache/openejb/util/Logger.java
index b32026b..605db83 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/util/Logger.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/util/Logger.java
@@ -22,6 +22,7 @@ import org.apache.openejb.loader.SystemInstance;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.Serializable;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.List;
@@ -231,13 +232,10 @@ public class Logger {
     /**
      * Builds a Logger object and returns it
      */
-    private static final Computable<Object[], Logger> loggerResolver = new Computable<Object[],
Logger>() {
+    private static final Computable<LoggerKey, Logger> loggerResolver = new Computable<LoggerKey,
Logger>() {
         @Override
-        public Logger compute(final Object[] args) throws InterruptedException {
-            final LogCategory category = (LogCategory) args[0];
-            final LogStream logStream = logStreamFactory.createLogStream(category);
-            final String baseName = (String) args[1];
-            return new Logger(category, logStream, baseName);
+        public Logger compute(final LoggerKey args) throws InterruptedException {
+            return new Logger(args.category, logStreamFactory.createLogStream(args.category),
args.baseName);
         }
     };
 
@@ -254,22 +252,22 @@ public class Logger {
     /**
      * Cache of parent-child relationships between resource names
      */
-    private static final Computable<String, String> heirarchyCache = new Memoizer<String,
String>(heirarchyResolver);
+    private static final Computable<String, String> heirarchyCache = new Memoizer<>(heirarchyResolver);
 
     /**
      * Cache of ResourceBundles
      */
-    private static final Computable<String, ResourceBundle> bundleCache = new Memoizer<String,
ResourceBundle>(bundleResolver);
+    private static final Computable<String, ResourceBundle> bundleCache = new Memoizer<>(bundleResolver);
 
     /**
      * Cache of Loggers
      */
-    private static final Computable<Object[], Logger> loggerCache = new Memoizer<Object[],
Logger>(loggerResolver);
+    private static final Computable<LoggerKey, Logger> loggerCache = new Memoizer<>(loggerResolver);
 
     /**
      * Cache of MessageFormats
      */
-    private static final Computable<String, MessageFormat> messageFormatCache = new
Memoizer<String, MessageFormat>(messageFormatResolver);
+    private static final Computable<String, MessageFormat> messageFormatCache = new
Memoizer<>(messageFormatResolver);
 
     /**
      * Finds a Logger from the cache and returns it. If not found in cache then builds a
Logger and returns it.
@@ -282,7 +280,7 @@ public class Logger {
         configure();
 
         try {
-            return loggerCache.compute(new Object[]{category, baseName});
+            return loggerCache.compute(new LoggerKey(category, baseName));
         } catch (final InterruptedException e) {
             // Don't return null here. Just create a new Logger and set it up.
             // It will not be stored in the cache, but a later lookup for the
@@ -697,4 +695,36 @@ public class Logger {
         return key;
     }
 
+    protected static class LoggerKey implements Serializable {
+        protected final LogCategory category;
+        protected final String baseName;
+        private final int hash;
+
+        protected LoggerKey(final LogCategory category, final String baseName) {
+            this.category = category;
+            this.baseName = baseName;
+
+            int result = category.hashCode();
+            hash = 31 * result + baseName.hashCode();
+        }
+
+        @Override
+        public boolean equals(final Object o) {
+            if (this == o) {
+                return true;
+            }
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+
+            final LoggerKey loggerKey = LoggerKey.class.cast(o);
+            return category.equals(loggerKey.category) && baseName.equals(loggerKey.baseName);
+
+        }
+
+        @Override
+        public int hashCode() {
+            return hash;
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/tomee/blob/c38893cb/container/openejb-core/src/main/java/org/apache/openejb/util/Memoizer.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/util/Memoizer.java b/container/openejb-core/src/main/java/org/apache/openejb/util/Memoizer.java
index a94df8c..7b1c035 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/util/Memoizer.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/util/Memoizer.java
@@ -69,4 +69,8 @@ public class Memoizer<K, V> implements Computable<K, V> {
             }
         }
     }
+
+    public ConcurrentMap<K, Future<V>> getCache() {
+        return cache;
+    }
 }

http://git-wip-us.apache.org/repos/asf/tomee/blob/c38893cb/container/openejb-core/src/test/java/org/apache/openejb/util/MemoizerTest.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/test/java/org/apache/openejb/util/MemoizerTest.java
b/container/openejb-core/src/test/java/org/apache/openejb/util/MemoizerTest.java
new file mode 100644
index 0000000..38ccbf4
--- /dev/null
+++ b/container/openejb-core/src/test/java/org/apache/openejb/util/MemoizerTest.java
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.util;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class MemoizerTest {
+    @Test
+    public void itLeaksByDesignWithWrongEqualsHashCodeAsKey() throws InterruptedException
{
+        final Memoizer<Object[], String> objectArrayCache = new Memoizer<>(new
Computable<Object[], String>() {
+            @Override
+            public String compute(final Object[] key) throws InterruptedException {
+                return key[0].toString();
+            }
+        });
+        objectArrayCache.compute(new Object[]{"1"});
+        assertEquals(1, objectArrayCache.getCache().size());
+
+        // this is a leak but the way it works, NEVER use an array as key without wrapping
it worse case in a list!
+        objectArrayCache.compute(new Object[]{"1"});
+        assertEquals(2, objectArrayCache.getCache().size());
+    }
+
+    @Test
+    public void noLeakWithLoggerKey() throws InterruptedException {
+        final Memoizer<Logger.LoggerKey, String> objectArrayCache = new Memoizer<>(new
Computable<Logger.LoggerKey, String>() {
+            @Override
+            public String compute(final Logger.LoggerKey key) throws InterruptedException
{
+                return key.baseName;
+            }
+        });
+        objectArrayCache.compute(new Logger.LoggerKey(LogCategory.OPENEJB.createChild("test"),
"MemoizerTest"));
+        assertEquals(1, objectArrayCache.getCache().size());
+
+        // this is a leak but the way it works
+        objectArrayCache.compute(new Logger.LoggerKey(LogCategory.OPENEJB.createChild("test"),
"MemoizerTest") /*new instance*/);
+        assertEquals(1, objectArrayCache.getCache().size());
+    }
+}


Mime
View raw message