#2259 Fix potential thread safe problem in InterceptorScope

profiler/src/main/java/com/navercorp/pinpoint/profiler/context/scope/ConcurrentPool.java

@@ -0,0 +1,68 @@
+/*
+ * Copyright 2016 NAVER Corp.
+ *
+ * Licensed 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 com.navercorp.pinpoint.profiler.context.scope;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public class ConcurrentPool<K, V> implements Pool<K, V> {
+
+    private final ConcurrentMap<K, V> pool = new ConcurrentHashMap<K, V>();
+
+    private final PoolObjectFactory<K, V> objectFactory;
+
+    public ConcurrentPool(PoolObjectFactory<K, V> objectFactory) {
+        if (objectFactory == null) {
+            throw new NullPointerException("objectFactory must not be null");
+        }
+        this.objectFactory = objectFactory;
+    }
+
+    @Override
+    public V get(K key) {
+        if (key == null) {
+            throw new IllegalArgumentException("key must not be null");
+        }
+
+        return pool.get(key);
+    }
+
+    @Override
+    public V add(K key) {
+        if (key == null) {
+            throw new IllegalArgumentException("name must not be null");
+        }
+
+        final V alreadyExist = this.pool.get(key);
+        if (alreadyExist != null) {
+            return alreadyExist;
+        }
+
+        final V newValue = this.objectFactory.create(key);
+        final V oldValue = this.pool.putIfAbsent(key, newValue);
+        if (oldValue != null) {
+            return oldValue;
+        }
+        return newValue;
+    }
+
+
+}

profiler/src/main/java/com/navercorp/pinpoint/profiler/context/scope/InterceptorScopeFactory.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright 2016 NAVER Corp.
+ *
+ * Licensed 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 com.navercorp.pinpoint.profiler.context.scope;
+
+import com.navercorp.pinpoint.bootstrap.interceptor.scope.InterceptorScope;
+import com.navercorp.pinpoint.profiler.interceptor.scope.DefaultInterceptorScope;
+
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public class InterceptorScopeFactory implements PoolObjectFactory<String, InterceptorScope> {
+    @Override
+    public InterceptorScope create(String scopeName) {
+        return new DefaultInterceptorScope(scopeName);
+    }
+}

profiler/src/main/java/com/navercorp/pinpoint/profiler/context/scope/Pool.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2016 NAVER Corp.
+ *
+ * Licensed 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 com.navercorp.pinpoint.profiler.context.scope;
+
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public interface Pool<K, V> {
+
+    V get(K key);
+
+    V add(K key);
+
+}

profiler/src/main/java/com/navercorp/pinpoint/profiler/context/scope/PoolObjectFactory.java

@@ -0,0 +1,25 @@
+/*
+ * Copyright 2016 NAVER Corp.
+ *
+ * Licensed 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 com.navercorp.pinpoint.profiler.context.scope;
+
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public interface PoolObjectFactory<K, V> {
+    V create(K key);
+}

profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/DefaultProfilerPluginContext.java

@@ -34,11 +34,13 @@
 import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPluginSetupContext;
 import com.navercorp.pinpoint.profiler.DefaultAgent;
 import com.navercorp.pinpoint.profiler.DynamicTransformService;
+import com.navercorp.pinpoint.profiler.context.scope.ConcurrentPool;
+import com.navercorp.pinpoint.profiler.context.scope.InterceptorScopeFactory;
+import com.navercorp.pinpoint.profiler.context.scope.Pool;
 import com.navercorp.pinpoint.profiler.instrument.ClassInjector;
 import com.navercorp.pinpoint.profiler.instrument.PluginClassInjector;
-import com.navercorp.pinpoint.profiler.interceptor.scope.DefaultInterceptorScope;
 import com.navercorp.pinpoint.profiler.util.JavaAssistUtils;
-import com.navercorp.pinpoint.profiler.util.NameValueList;
+
 
 public class DefaultProfilerPluginContext implements ProfilerPluginSetupContext, InstrumentContext {
     private final DefaultAgent agent;
@@ -47,7 +49,7 @@ public class DefaultProfilerPluginContext implements ProfilerPluginSetupContext,
     private final List<ApplicationTypeDetector> serverTypeDetectors = new ArrayList<ApplicationTypeDetector>();
     private final List<ClassFileTransformer> classTransformers = new ArrayList<ClassFileTransformer>();
 
-    private final NameValueList<InterceptorScope> interceptorScopeList = new NameValueList<InterceptorScope>();
+    private final Pool<String, InterceptorScope> interceptorScopePool = new ConcurrentPool<String, InterceptorScope>(new InterceptorScopeFactory());
 
     public DefaultProfilerPluginContext(DefaultAgent agent, ClassInjector classInjector) {
         if (agent == null) {
@@ -187,13 +189,7 @@ public InterceptorScope getInterceptorScope(String name) {
         if (name == null) {
             throw new NullPointerException("name must not be null");
         }
-        InterceptorScope scope = interceptorScopeList.get(name);
-
-        if (scope == null) {
-            scope = new DefaultInterceptorScope(name);
-            interceptorScopeList.add(name, scope);
-        }
-
-        return scope;
+
+        return interceptorScopePool.add(name);
     }
 }

profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/xml/interceptor/AnnotatedInterceptorInjector.java

@@ -60,7 +60,7 @@ private InterceptorScope getScope() {
         if (scopeName == null) {
             return null;
         } else {
-            return  pluginContext.getInterceptorScope(scopeName);
+            return pluginContext.getInterceptorScope(scopeName);
         }
     }
 

profiler/src/main/java/com/navercorp/pinpoint/profiler/util/NameValueList.java

@@ -15,13 +15,14 @@
 package com.navercorp.pinpoint.profiler.util;
 
 import java.util.ArrayList;
+import java.util.List;
 
 /**
  * @author Jongho Moon
  *
  */
 public class NameValueList<T> {
-    private final ArrayList<NameValue<T>> list;
+    private final List<NameValue<T>> list;
 
     public NameValueList() {
         this(0);

profiler/src/test/java/com/navercorp/pinpoint/profiler/context/scope/ConcurrentPoolTest.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2016 NAVER Corp.
+ *
+ * Licensed 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 com.navercorp.pinpoint.profiler.context.scope;
+
+import com.navercorp.pinpoint.bootstrap.interceptor.scope.InterceptorScope;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public class ConcurrentPoolTest {
+    @Test
+    public void testConcurrentPool() throws Exception {
+
+        InterceptorScopeFactory traceScopeFactory = new InterceptorScopeFactory();
+        Pool<String, InterceptorScope> pool = new ConcurrentPool<String, InterceptorScope>(traceScopeFactory);
+
+        final String OBJECT_NAME = "test";
+
+        Assert.assertNull(pool.get(OBJECT_NAME));
+
+        InterceptorScope addedScope = pool.add(OBJECT_NAME);
+        Assert.assertSame(pool.get(OBJECT_NAME), addedScope);
+
+        InterceptorScope exist = pool.add(OBJECT_NAME);
+        Assert.assertSame(exist, addedScope);
+    }
+
+}