Repository: pinpoint-apm/pinpoint
Issue: fix unsafe resource cleanup of profiler
Labels: bug


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/LoggingInterceptor.java b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/LoggingInterceptor.java
index 9f072384ecb5..d1525d3d1b77 100644
--- a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/LoggingInterceptor.java
+++ b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/LoggingInterceptor.java
@@ -23,7 +23,7 @@
 /**
  * @author emeroad
  */
-public class LoggingInterceptor implements StaticAroundInterceptor, AroundInterceptor {
+public class LoggingInterceptor implements StaticAroundInterceptor, AroundInterceptor, AroundInterceptor0, AroundInterceptor1, AroundInterceptor2, AroundInterceptor3, AroundInterceptor4, AroundInterceptor5, ApiIdAwareAroundInterceptor {
 
     private final Logger logger;
 
@@ -63,4 +63,72 @@ public static String defaultString(final Object object) {
         return String.valueOf(object);
     }
 
+    @Override
+    public void before(Object target, int apiId, Object[] args) {
+    }
+
+    @Override
+    public void after(Object target, int apiId, Object[] args, Object result, Throwable throwable) {
+
+    }
+
+    @Override
+    public void before(Object target) {
+
+    }
+
+    @Override
+    public void after(Object target, Object result, Throwable throwable) {
+
+    }
+
+    @Override
+    public void before(Object target, Object arg0) {
+
+    }
+
+    @Override
+    public void after(Object target, Object arg0, Object result, Throwable throwable) {
+
+    }
+
+    @Override
+    public void before(Object target, Object arg0, Object arg1) {
+
+    }
+
+    @Override
+    public void after(Object target, Object arg0, Object arg1, Object result, Throwable throwable) {
+
+    }
+
+    @Override
+    public void before(Object target, Object arg0, Object arg1, Object arg2) {
+
+    }
+
+    @Override
+    public void after(Object target, Object arg0, Object arg1, Object arg2, Object result, Throwable throwable) {
+
+    }
+
+    @Override
+    public void before(Object target, Object arg0, Object arg1, Object arg2, Object arg3) {
+
+    }
+
+    @Override
+    public void after(Object target, Object arg0, Object arg1, Object arg2, Object arg3, Object result, Throwable throwable) {
+
+    }
+
+    @Override
+    public void before(Object target, Object arg0, Object arg1, Object arg2, Object arg3, Object arg4) {
+
+    }
+
+    @Override
+    public void after(Object target, Object arg0, Object arg1, Object arg2, Object arg3, Object arg4, Object result, Throwable throwable) {
+
+    }
 }
diff --git a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/DefaultInterceptorRegistryAdaptor.java b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/DefaultInterceptorRegistryAdaptor.java
index eee307ca0fcb..970b2bc578bc 100644
--- a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/DefaultInterceptorRegistryAdaptor.java
+++ b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/DefaultInterceptorRegistryAdaptor.java
@@ -11,8 +11,6 @@
 public final class DefaultInterceptorRegistryAdaptor implements InterceptorRegistryAdaptor {
     private static final LoggingInterceptor LOGGING_INTERCEPTOR = new LoggingInterceptor("com.navercorp.pinpoint.profiler.interceptor.LOGGING_INTERCEPTOR");
 
-    public static final InterceptorRegistry REGISTRY = new InterceptorRegistry();
-
     private final static int DEFAULT_MAX = 4096;
     private final int registrySize;
 
@@ -52,7 +50,11 @@ private int nextId() {
     }
 
     public Interceptor getInterceptor(int key) {
-        Interceptor interceptor = this.index.get(key);
-        return interceptor == null ? LOGGING_INTERCEPTOR : interceptor;
+        final Interceptor interceptor = this.index.get(key);
+        if (interceptor == null) {
+            return LOGGING_INTERCEPTOR;
+        } else {
+            return interceptor;
+        }
     }
 }
diff --git a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/EmptyRegistryAdaptor.java b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/EmptyRegistryAdaptor.java
new file mode 100644
index 000000000000..c98dbec26cb4
--- /dev/null
+++ b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/EmptyRegistryAdaptor.java
@@ -0,0 +1,45 @@
+/*
+ * *
+ *  * Copyright 2014 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.bootstrap.interceptor.registry;
+
+import com.navercorp.pinpoint.bootstrap.interceptor.Interceptor;
+import com.navercorp.pinpoint.bootstrap.interceptor.LoggingInterceptor;
+
+/**
+ * @author emeroad
+ */
+public final class EmptyRegistryAdaptor implements InterceptorRegistryAdaptor {
+
+    public static final InterceptorRegistryAdaptor EMPTY = new EmptyRegistryAdaptor();
+
+    private static final LoggingInterceptor LOGGING_INTERCEPTOR = new LoggingInterceptor("com.navercorp.pinpoint.profiler.interceptor.EMPTY");
+
+    public EmptyRegistryAdaptor() {
+    }
+
+
+    @Override
+    public int addInterceptor(Interceptor interceptor) {
+        return -1;
+    }
+
+
+    public Interceptor getInterceptor(int key) {
+        return LOGGING_INTERCEPTOR;
+    }
+}
diff --git a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/InterceptorRegistry.java b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/InterceptorRegistry.java
index bd070556817e..e12ef7c61e0d 100644
--- a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/InterceptorRegistry.java
+++ b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/interceptor/registry/InterceptorRegistry.java
@@ -26,7 +26,7 @@ public final class InterceptorRegistry {
 
     private static final Locker LOCK = new DefaultLocker();
 
-    private static InterceptorRegistryAdaptor REGISTRY;
+    private static InterceptorRegistryAdaptor REGISTRY = EmptyRegistryAdaptor.EMPTY;
 
     public static void bind(final InterceptorRegistryAdaptor interceptorRegistryAdaptor, final Object lock) {
         if (interceptorRegistryAdaptor == null) {
@@ -42,7 +42,7 @@ public static void bind(final InterceptorRegistryAdaptor interceptorRegistryAdap
 
     public static void unbind(final Object lock) {
         if (LOCK.unlock(lock)) {
-            REGISTRY = null;
+            REGISTRY = EmptyRegistryAdaptor.EMPTY;
         } else {
             throw new IllegalStateException("unbind failed.");
         }
diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/DefaultAgent.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/DefaultAgent.java
index 6a68fdfbb50a..a25f5b714e38 100644
--- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/DefaultAgent.java
+++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/DefaultAgent.java
@@ -390,6 +390,10 @@ public void start() {
 
     @Override
     public void stop() {
+        stop(false);
+    }
+
+    public void stop(boolean staticResourceCleanup) {
         synchronized (this) {
             if (this.agentStatus == AgentStatus.RUNNING) {
                 changeStatus(AgentStatus.STOPPED);
@@ -408,9 +412,11 @@ public void stop() {
         this.statDataSender.stop();
 
         closeTcpDataSender();
-
-        PLoggerFactory.unregister(this.binder);
-        this.interceptorRegistryBinder.unbind();
+        // for testcase
+        if (staticResourceCleanup) {
+            PLoggerFactory.unregister(this.binder);
+            this.interceptorRegistryBinder.unbind();
+        }
     }
 
     private void closeTcpDataSender() {


Test patch:
diff --git a/profiler/src/main/java/com/navercorp/pinpoint/test/junit4/TestContext.java b/profiler/src/main/java/com/navercorp/pinpoint/test/junit4/TestContext.java
index f4f8f3984d9c..6147b72306c7 100644
--- a/profiler/src/main/java/com/navercorp/pinpoint/test/junit4/TestContext.java
+++ b/profiler/src/main/java/com/navercorp/pinpoint/test/junit4/TestContext.java
@@ -88,7 +88,7 @@ public Class<?> getBaseTestClass() {
     @Override
     public void close() throws IOException {
         if (mockAgent != null) {
-            mockAgent.stop();
+            mockAgent.stop(true);
         }
         PLoggerFactory.unregister(loggerBinder);
     }