Repository: pinpoint-apm/pinpoint
Issue: IllegalAccessError in Mariadb jdbc url parser
Labels: bug
module:plugin

#### Description
There has been an [issue](https://groups.google.com/forum/#!topic/pinpoint_user/0-Wq1guD4wo) where Mariadb jdbc url would show up as *Unknown* due to **IllegalAccessError** on **MariaDBJdbcUrlParser$Type** as below.
```
public class MariaDBJdbcUrlParser implements JdbcUrlParserV2 {
    @Override
    public DatabaseInfo parse(String jdbcUrl) {
        Type type = Type.findType(jdbcUrl);
        ...
    }        
    private enum Type {
        ...
    }
}
```
This is due to a difference in class load timing between **MariaDBJdbcUrlParser** and it's nested **MariaDBJdbcUrlParser$Type** when Mariadb Java driver is loaded directly by the system class loader (ie is in the system class path).
**MariaDBJdbcUrlParser** is loaded by pinpoint's plugin class loader before application startup and **MariaDBJdbcUrlParser$Type** is loaded only when it is first referenced (ie `Type type = Type.findType(jdbcUrl)`).
At this point, the system class loader would have Mariadb plugin in it's search path (in order to load the interceptors for transformation), so **MariaDBJdbcUrlParser$Type** is loaded by the system class loader.

However, due to the following, an **IllegalAccessError** would be thrown.
> At run time, a class or interface is determined not by its name alone, but by a pair: its binary name and its defining class loader. Each such class or interface belongs to a single run-time package. The run-time package of a class or interface is determined by the package name and defining class loader of the class or interface.
https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-5.html#jvms-5.3

> A class or interface C is accessible to a class or interface D if and only if either of the following is true: C is public, C and D are members of the same run-time package
https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-5.html#jvms-5.4.4

The issue did not manifest itself when ran on Tomcat as Tomcat has it's own class loader hierarchy and Mariadb plugin was not added to the system class loader's search path, but was instead added to Tomcat's webapp class loader's search path, and so the loading of **MariaDBJdbcUrlParser$Type** could not have been delegated to system class loader.

Fix would be to load **MariaDBJdbcUrlParser$Type** as well when **MariaDBJdbcUrlParser** is loaded by pinpoint's plugin class loader.

Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/plugins/mariadb-jdbc/src/main/java/com/navercorp/pinpoint/plugin/jdbc/mariadb/MariaDBJdbcUrlParser.java b/plugins/mariadb-jdbc/src/main/java/com/navercorp/pinpoint/plugin/jdbc/mariadb/MariaDBJdbcUrlParser.java
index e47c3e11033b..881f9bbb75a8 100644
--- a/plugins/mariadb-jdbc/src/main/java/com/navercorp/pinpoint/plugin/jdbc/mariadb/MariaDBJdbcUrlParser.java
+++ b/plugins/mariadb-jdbc/src/main/java/com/navercorp/pinpoint/plugin/jdbc/mariadb/MariaDBJdbcUrlParser.java
@@ -27,7 +27,9 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.EnumSet;
 import java.util.List;
+import java.util.Set;
 
 /**
  * @author dawidmalina
@@ -36,10 +38,11 @@ public class MariaDBJdbcUrlParser implements JdbcUrlParserV2 {
 
 //    jdbc:(mysql|mariadb):[replication:|failover|loadbalance:|aurora:]//<hostDescription>[,<hostDescription>]/[database>]
 //    jdbc:mariadb:loadbalance://10.22.33.44:3306,10.22.33.55:3306/MariaDB?characterEncoding=UTF-8
-    private static final String URL_PREFIX = "jdbc:mariadb:";
-
+    private static final String MARIA_URL_PREFIX = "jdbc:mariadb:";
     private static final String MYSQL_URL_PREFIX = "jdbc:mysql:";
 
+    private static final Set<Type> TYPES = EnumSet.allOf(Type.class);
+
     private final PLogger logger = PLoggerFactory.getLogger(this.getClass());
 
     @Override
@@ -49,20 +52,18 @@ public DatabaseInfo parse(String jdbcUrl) {
             return UnKnownDatabaseInfo.INSTANCE;
         }
 
-        Type type = Type.findType(jdbcUrl);
+        Type type = getType(jdbcUrl);
         if (type == null) {
-            logger.info("jdbcUrl has invalid prefix.(url:{}, prefix:{}, {})", jdbcUrl, URL_PREFIX, MYSQL_URL_PREFIX);
+            logger.info("jdbcUrl has invalid prefix.(url:{}, valid prefixes:{}, {})", jdbcUrl, MARIA_URL_PREFIX, MYSQL_URL_PREFIX);
             return UnKnownDatabaseInfo.INSTANCE;
         }
 
-        DatabaseInfo result = null;
         try {
-            result = parse0(jdbcUrl, type);
+            return parse0(jdbcUrl, type);
         } catch (Exception e) {
             logger.info("MaridDBJdbcUrl parse error. url: {}, Caused: {}", jdbcUrl, e.getMessage(), e);
-            result = UnKnownDatabaseInfo.createUnknownDataBase(MariaDBConstants.MARIADB, MariaDBConstants.MARIADB_EXECUTE_QUERY, jdbcUrl);
+            return UnKnownDatabaseInfo.createUnknownDataBase(MariaDBConstants.MARIADB, MariaDBConstants.MARIADB_EXECUTE_QUERY, jdbcUrl);
         }
-        return result;
     }
 
     private DatabaseInfo parse0(String url, Type type) {
@@ -119,8 +120,17 @@ public ServiceType getServiceType() {
         return MariaDBConstants.MARIADB;
     }
 
-    private static enum Type {
-        MARIA(URL_PREFIX),
+    private static Type getType(String jdbcUrl) {
+        for (Type type : TYPES) {
+            if (jdbcUrl.startsWith(type.getUrlPrefix())) {
+                return type;
+            }
+        }
+        return null;
+    }
+
+    private enum Type {
+        MARIA(MARIA_URL_PREFIX),
         MYSQL(MYSQL_URL_PREFIX);
 
         private final String urlPrefix;
@@ -136,19 +146,7 @@ private String getUrlPrefix() {
         }
 
         private String getLoadbalanceUrlPrefix() {
-            return urlPrefix + "loadbalance:";
+            return loadbalanceUrlPrefix;
         }
-
-        private static Type findType(String jdbcUrl) {
-            for (Type type : Type.values()) {
-                if (jdbcUrl.startsWith(type.urlPrefix)) {
-                    return type;
-                }
-            }
-
-            return null;
-        }
-
     }
-
 }


Test patch: