async-profiler#1174: Handle feedback & remove dependency on java version

Author: Baraa-Hasheesh

src/vmEntry.cpp

@@ -106,48 +106,32 @@ static void* resolveMethodIdEnd() {
     return NULL;
 }
 
-void VM::initWrapper(JavaVM* vm) {
-    JNIEnv* env;
-
-    jint get_env_result = vm->GetEnv((void**)&env, JNI_VERSION_1_6);
-    if (get_env_result == JNI_OK) {
-        VM::init(vm, true);
-        return;
-    } else if (get_env_result != JNI_EDETACHED) {
-        return;
-    }
-
-    // VM exists but not attached to current thread    
-
-    // Work around for JDK-8308341, as JNI might return a none initialized VM so we check for the existence of certain VM threads 
-    // "VM Thread" and "Service Thread" in this case are used as indication that the VM is in a good enough state to be used
-    char thread_name[4096];
+// Work around for JDK-8308341, as JNI might return a none initialized VM so we check for the existence of certain VM threads 
+// "VM Thread" and "Service Thread" in this case are used as indication that the VM is in a good enough state to be used
+bool VM::checkJvmThreads() {
+    char thread_name[20];
     int thread_name_offset = 0;
     bool vm_thread = false, service_thread = false;
 
     ThreadList* list = OS::listThreads();
     while (list->hasNext()) {
-        if (!OS::threadName(list->next(), thread_name, 4096)) {
+        if (!OS::threadName(list->next(), thread_name, sizeof(thread_name))) {
             continue;
         }
 
         // For MacOs Thread name starts with "Java: "
-        if (!OS::isLinux() && thread_name[0] == 'J' && strncmp(thread_name, "Java: ", 6) == 0) {
+        if (strncmp(thread_name, "Java: ", 6) == 0) {
             thread_name_offset = 6;
         }
 
-        if (thread_name[thread_name_offset] == 'V' && strcmp(thread_name + thread_name_offset, "VM Thread") == 0) {
+        if (strcmp(thread_name + thread_name_offset, "VM Thread") == 0) {
             vm_thread = true;
-        } else if (thread_name[thread_name_offset] == 'S' && strcmp(thread_name + thread_name_offset, "Service Thread") == 0) {
+        } else if (strcmp(thread_name + thread_name_offset, "Service Thread") == 0) {
             service_thread = true;
         }
     }
 
-    // Check that VM is in a good state to attach the thread
-    if (service_thread && vm_thread && vm->AttachCurrentThreadAsDaemon((void**)&env, NULL) == JNI_OK) {
-        VM::init(vm, true);
-        return;
-    }
+    return vm_thread && service_thread;
 }
 
 bool VM::init(JavaVM* vm, bool attach) {
@@ -514,6 +498,7 @@ JNI_OnUnload(JavaVM* vm, void* reserved) {
 // Try to find a running JVM instance & attach it to the profiler
 void VM::tryAttach() {
     JavaVM* jvm;
+    JNIEnv* env;
     jsize nVMs;
 
     if (_getJvm == NULL) {
@@ -530,5 +515,18 @@ void VM::tryAttach() {
     if (result != JNI_OK || nVMs != 1) {
         return;
     }
-    VM::initWrapper(jvm);
+
+    // Check the current JNI environment status
+    jint get_env_result = jvm->GetEnv((void**)&env, JNI_VERSION_1_6);
+    if (get_env_result == JNI_OK) {
+        VM::init(jvm, true);
+        return;
+    } else if (get_env_result != JNI_EDETACHED) {
+        return;
+    }
+
+    // Here JVM exists but is not attached to current thread    
+    if (VM::checkJvmThreads() && jvm->AttachCurrentThreadAsDaemon((void**)&env, NULL) == JNI_OK) {
+        VM::init(jvm, true);
+    }
 }

src/vmEntry.h

@@ -113,7 +113,7 @@ class VM {
     static void loadMethodIDs(jvmtiEnv* jvmti, JNIEnv* jni, jclass klass);
     static void loadAllMethodIDs(jvmtiEnv* jvmti, JNIEnv* jni);
 
-    static void initWrapper(JavaVM* vm);
+    static bool checkJvmThreads();
 
   public:
     static AsyncGetCallTrace _asyncGetCallTrace;

test/one/profiler/test/TestProcess.java

@@ -34,7 +34,6 @@ public class TestProcess implements Closeable {
     public static final String TESTBIN = "%testbin";
 
     private static final String JAVA_HOME = System.getProperty("java.home");
-    private static final String JAVA_VERSION = Runner.detectJvmVersion() + "";
 
     private static final Pattern filePattern = Pattern.compile("(%[a-z]+)(\\.[a-z]+)?");
 
@@ -90,7 +89,6 @@ public TestProcess(Test test, Os currentOs, String logDir) throws Exception {
             }
         }
         pb.environment().put("TEST_JAVA_HOME", JAVA_HOME);
-        pb.environment().put("TEST_JAVA_VERSION", JAVA_VERSION);
 
         this.p = pb.start();
 

test/test/nonjava/non_java_app.cpp

@@ -10,28 +10,16 @@
 #include <limits.h>
 #include "asprof.h"
 #include <pthread.h>
-
-// detect arch for java 8
-#if defined(__x86_64__) || defined(_M_X64)
-#  define JAVA8_ARCH_PATH "amd64/"
-#elif defined(__i386) || defined(_M_IX86)
-#  define JAVA8_ARCH_PATH "i386/"
-#elif defined(__aarch64__) || defined(_M_ARM64)
-#  define JAVA8_ARCH_PATH "aarch64/"
-#elif defined(__arm__) || defined(_M_ARM)
-#  define JAVA8_ARCH_PATH "arm/"
-#else
-#  define JAVA8_ARCH_PATH "/"
-#endif
+#include <time.h>
+#include <dirent.h>
+#include <string.h>
 
 #ifdef __linux__
 const char profiler_lib_path[] = "build/lib/libasyncProfiler.so";
-const char jvm_lib_path[] = "lib/server/libjvm.so";
-const char jvm8_lib_path[] = "lib/" JAVA8_ARCH_PATH "server/libjvm.so";
+const char jvm_lib_path[] = "server/libjvm.so";
 #else
 const char profiler_lib_path[] = "build/lib/libasyncProfiler.dylib";
-const char jvm_lib_path[] = "lib/server/libjvm.dylib";
-const char jvm8_lib_path[] = "lib/server/libjvm.dylib";
+const char jvm_lib_path[] = "server/libjvm.dylib";
 #endif
 
 typedef jint (*CreateJvm)(JavaVM **, void **, void *);
@@ -45,8 +33,6 @@ JNIEnv* _env;
 
 void* _jvm_lib;
 
-jint java_version = 0;
-
 void outputCallback(const char* buffer, size_t size) {
     fwrite(buffer, sizeof(char), size, stderr);
 }
@@ -84,30 +70,49 @@ void stopProfiler(char* outputFile) {
 }
 
 void loadJvmLib() {
+    char lib_path[PATH_MAX];    
+
     // Get Java home
     char* java_home = getenv("TEST_JAVA_HOME");
     if (java_home == NULL) {
         std::cerr << "TEST_JAVA_HOME is not set" << std::endl;
         exit(1);
     }
-    // Get Java version
-    char* version = getenv("TEST_JAVA_VERSION");
-    if (version == NULL) {
-        std::cerr << "TEST_JAVA_VERSION is not set" << std::endl;
-        exit(1);
+
+    // check that libjvm is found under the standard path
+    snprintf(lib_path, sizeof(lib_path), "%s/%s/%s", java_home, "lib", jvm_lib_path);
+    if ((_jvm_lib = dlopen(lib_path, RTLD_LOCAL | RTLD_NOW)) != NULL) {
+        return;
     }
-    // Java 8 or higher
-    java_version = std::stoi(version);
-    if (java_version < 8) {
-        std::cerr << "Unsupported Java version: " << version << std::endl;
+
+    char java_lib_home[PATH_MAX];
+    struct dirent* entry;
+    DIR* dir;
+
+    // libjvm wasn't found under standard path, this could happen in JDK 8 where the path is formated like:
+    // ${TEST_JAVA_HOME}/lib/${ARCH}/server/libjvm.(so|dylib)
+    snprintf(java_lib_home, sizeof(java_lib_home), "%s/lib", java_home);
+    dir = opendir(java_lib_home);
+    if (dir == NULL) {
+        std::cerr << "Error opening directory: " << java_lib_home << std::endl;
         exit(1);
     }
 
-    char lib_path[PATH_MAX];
-    snprintf(lib_path, sizeof(lib_path), "%s/%s", java_home, java_version == 8 ? jvm8_lib_path : jvm_lib_path);
-    _jvm_lib = dlopen(lib_path, RTLD_LOCAL | RTLD_NOW);
+    while((entry = readdir(dir)) != NULL) {
+        // Skip .. & .
+        if (strcmp(entry->d_name, "..") == 0 || strcmp(entry->d_name, ".") == 0) {
+            continue;
+        }
+       
+        snprintf(lib_path, sizeof(lib_path), "%s/%s/%s", java_lib_home, entry->d_name, jvm_lib_path);
+        if ((_jvm_lib = dlopen(lib_path, RTLD_LOCAL | RTLD_NOW)) != NULL) {
+            break;
+        }
+    }
+
+    // libjvm was never found
     if (_jvm_lib == NULL) {
-        std::cerr << "Unable to find: " << lib_path << ", Error: " << dlerror() << std::endl;
+        std::cerr << "Unable to find: libjvm" << std::endl;
         exit(1);
     }
 }
@@ -317,6 +322,8 @@ The profiler should be able to profile the JVM task
 The JVM is loaded and started before the profiling session is started so it's attached correctly at the session start
 */
 void testFlow4(int argc, char** argv) {
+    struct timespec wait_time = {(time_t)(2), 0L};
+
     validateArgsCount(argc, 3, "Minimum Arguments is 2");
 
     loadProfiler();
@@ -325,12 +332,12 @@ void testFlow4(int argc, char** argv) {
     pthread_create(&thread, NULL, jvmThreadWrapper, NULL);
 
     // busy wait for JVM to start on the thread
-    for (int i = 0; i < 3; i++) for (int i = 0; i < 1000000000; ++i) {}
+    nanosleep(&wait_time, NULL);
 
     startProfiler();
 
     // busy wait for profiler to sample JVM
-    for (int i = 0; i < 3; i++) for (int i = 0; i < 1000000000; ++i) {}
+    nanosleep(&wait_time, NULL);
 
     stopProfiler(argv[2]);
 }